ed1787d
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
ed1787d
From: Daniel Axtens <dja@axtens.net>
ed1787d
Date: Thu, 25 Nov 2021 02:22:48 +1100
ed1787d
Subject: [PATCH] mm: Document grub_free()
ed1787d
ed1787d
The grub_free() possesses a surprising number of quirks, and also
ed1787d
uses single-letter variable names confusingly to iterate through
ed1787d
the free list.
ed1787d
ed1787d
Document what's going on.
ed1787d
ed1787d
Use prev and cur to iterate over the free list.
ed1787d
ed1787d
Signed-off-by: Daniel Axtens <dja@axtens.net>
ed1787d
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
ed1787d
(cherry picked from commit 1f8d0b01738e49767d662d6426af3570a64565f0)
ed1787d
---
ed1787d
 grub-core/kern/mm.c | 63 ++++++++++++++++++++++++++++++++++-------------------
ed1787d
 1 file changed, 41 insertions(+), 22 deletions(-)
ed1787d
ed1787d
diff --git a/grub-core/kern/mm.c b/grub-core/kern/mm.c
ed1787d
index db7e0b2a5b..0351171cf9 100644
ed1787d
--- a/grub-core/kern/mm.c
ed1787d
+++ b/grub-core/kern/mm.c
ed1787d
@@ -446,54 +446,73 @@ grub_free (void *ptr)
ed1787d
     }
ed1787d
   else
ed1787d
     {
ed1787d
-      grub_mm_header_t q, s;
ed1787d
+      grub_mm_header_t cur, prev;
ed1787d
 
ed1787d
 #if 0
ed1787d
-      q = r->first;
ed1787d
+      cur = r->first;
ed1787d
       do
ed1787d
 	{
ed1787d
 	  grub_printf ("%s:%d: q=%p, q->size=0x%x, q->magic=0x%x\n",
ed1787d
-		       GRUB_FILE, __LINE__, q, q->size, q->magic);
ed1787d
-	  q = q->next;
ed1787d
+		       GRUB_FILE, __LINE__, cur, cur->size, cur->magic);
ed1787d
+	  cur = cur->next;
ed1787d
 	}
ed1787d
-      while (q != r->first);
ed1787d
+      while (cur != r->first);
ed1787d
 #endif
ed1787d
-
ed1787d
-      for (s = r->first, q = s->next; q <= p || q->next >= p; s = q, q = s->next)
ed1787d
+      /* Iterate over all blocks in the free ring.
ed1787d
+       *
ed1787d
+       * The free ring is arranged from high addresses to low
ed1787d
+       * addresses, modulo wraparound.
ed1787d
+       *
ed1787d
+       * We are looking for a block with a higher address than p or
ed1787d
+       * whose next address is lower than p.
ed1787d
+       */
ed1787d
+      for (prev = r->first, cur = prev->next; cur <= p || cur->next >= p;
ed1787d
+	   prev = cur, cur = prev->next)
ed1787d
 	{
ed1787d
-	  if (q->magic != GRUB_MM_FREE_MAGIC)
ed1787d
-	    grub_fatal ("free magic is broken at %p: 0x%x", q, q->magic);
ed1787d
+	  if (cur->magic != GRUB_MM_FREE_MAGIC)
ed1787d
+	    grub_fatal ("free magic is broken at %p: 0x%x", cur, cur->magic);
ed1787d
 
ed1787d
-	  if (q <= q->next && (q > p || q->next < p))
ed1787d
+	  /* Deal with wrap-around */
ed1787d
+	  if (cur <= cur->next && (cur > p || cur->next < p))
ed1787d
 	    break;
ed1787d
 	}
ed1787d
 
ed1787d
+      /* mark p as free and insert it between cur and cur->next */
ed1787d
       p->magic = GRUB_MM_FREE_MAGIC;
ed1787d
-      p->next = q->next;
ed1787d
-      q->next = p;
ed1787d
+      p->next = cur->next;
ed1787d
+      cur->next = p;
ed1787d
 
ed1787d
+      /*
ed1787d
+       * If the block we are freeing can be merged with the next
ed1787d
+       * free block, do that.
ed1787d
+       */
ed1787d
       if (p->next + p->next->size == p)
ed1787d
 	{
ed1787d
 	  p->magic = 0;
ed1787d
 
ed1787d
 	  p->next->size += p->size;
ed1787d
-	  q->next = p->next;
ed1787d
+	  cur->next = p->next;
ed1787d
 	  p = p->next;
ed1787d
 	}
ed1787d
 
ed1787d
-      r->first = q;
ed1787d
+      r->first = cur;
ed1787d
 
ed1787d
-      if (q == p + p->size)
ed1787d
+      /* Likewise if can be merged with the preceeding free block */
ed1787d
+      if (cur == p + p->size)
ed1787d
 	{
ed1787d
-	  q->magic = 0;
ed1787d
-	  p->size += q->size;
ed1787d
-	  if (q == s)
ed1787d
-	    s = p;
ed1787d
-	  s->next = p;
ed1787d
-	  q = s;
ed1787d
+	  cur->magic = 0;
ed1787d
+	  p->size += cur->size;
ed1787d
+	  if (cur == prev)
ed1787d
+	    prev = p;
ed1787d
+	  prev->next = p;
ed1787d
+	  cur = prev;
ed1787d
 	}
ed1787d
 
ed1787d
-      r->first = q;
ed1787d
+      /*
ed1787d
+       * Set r->first such that the just free()d block is tried first.
ed1787d
+       * (An allocation is tried from *first->next, and cur->next == p.)
ed1787d
+       */
ed1787d
+      r->first = cur;
ed1787d
     }
ed1787d
 }
ed1787d