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?
- 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>