Re: [PATCH 1/2] mm/hugetlb: compute/return the number of regions added by region_add()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/21/2015 04:30 PM, Andrew Morton wrote:
On Mon, 18 May 2015 10:49:08 -0700 Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:

Modify region_add() to keep track of regions(pages) added to the
reserve map and return this value.  The return value can be
compared to the return value of region_chg() to determine if the
map was modified between calls.  Make vma_commit_reservation()
also pass along the return value of region_add().  The special
case return values of vma_needs_reservation() should also be
taken into account when determining the return value of
vma_commit_reservation().

Could we please get this code slightly documented while it's hot in
your mind?

Will do.  I'll provide an updated patch to address your questions and
better document this whole region/reserve map stuff.  It took me a few
days to wrap my head around how it actually works.

--
Mike Kravetz

- One has to do an extraordinary amount of reading to discover that
   the units of file_region.from and .to are "multiples of
   1<<huge_page_order(h)" (where "h" is imponderable).

   Let's get this written down?

- Is file_region.to inclusive or exclusive?

- What are they called "from" and "to" anyway?  We usually use
   "start" and "end" for such things.


--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -156,6 +156,7 @@ static long region_add(struct resv_map *resv, long f, long t)
  {
  	struct list_head *head = &resv->regions;
  	struct file_region *rg, *nrg, *trg;
+	long chg = 0;

  	spin_lock(&resv->lock);
  	/* Locate the region we are either in or before. */
@@ -181,14 +182,17 @@ static long region_add(struct resv_map *resv, long f, long t)
  		if (rg->to > t)
  			t = rg->to;
  		if (rg != nrg) {
+			chg -= (rg->to - rg->from);
  			list_del(&rg->link);
  			kfree(rg);
  		}
  	}
+	chg += (nrg->from - f);
  	nrg->from = f;
+	chg += t - nrg->to;
  	nrg->to = t;
  	spin_unlock(&resv->lock);
-	return 0;
+	return chg;
  }

Let's document the return value.  It appears that this function is
designed to return a negative number (units?) on a successful addition.
Why, and what does that number represent.


  static long region_chg(struct resv_map *resv, long f, long t)
@@ -1349,18 +1353,25 @@ static long vma_needs_reservation(struct hstate *h,
  	else
  		return chg < 0 ? chg : 0;
  }
-static void vma_commit_reservation(struct hstate *h,
+
+static long vma_commit_reservation(struct hstate *h,
  			struct vm_area_struct *vma, unsigned long addr)
  {
  	struct resv_map *resv;
  	pgoff_t idx;
+	long add;

  	resv = vma_resv_map(vma);
  	if (!resv)
-		return;
+		return 1;

  	idx = vma_hugecache_offset(h, vma, addr);
-	region_add(resv, idx, idx + 1);
+	add = region_add(resv, idx, idx + 1);
+
+	if (vma->vm_flags & VM_MAYSHARE)
+		return add;
+	else
+		return 0;
  }

Let's document the return value here as well please.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]