On Mon, Jul 20, 2015 at 10:50:12AM -0700, Mike Kravetz wrote:
...
> > ...
> >> @@ -3236,11 +3360,14 @@ retry:
> >> * any allocations necessary to record that reservation occur outside
> >> * the spinlock.
> >> */
> >> - if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED))
> >> + if ((flags & FAULT_FLAG_WRITE) && !(vma->vm_flags & VM_SHARED)) {
> >> if (vma_needs_reservation(h, vma, address) < 0) {
> >> ret = VM_FAULT_OOM;
> >> goto backout_unlocked;
> >> }
> >> + /* Just decrements count, does not deallocate */
> >> + vma_abort_reservation(h, vma, address);
> >> + }
> >
> > This is not "abort reservation" operation, but you use "abort reservation"
> > routine, which might confusing and makes future maintenance hard. I think
> > this should be done in a simplified variant of vma_commit_reservation()
> > (maybe just an alias of your vma_abort_reservation()) or fast path in
> > vma_commit_reservation().
>
> I am struggling a bit with the names of these routines. The
> routines in question are:
>
> vma_needs_reservation - This is a wrapper for region_chg(), so the
> return value is the number of regions needed for the page.
> Since there is only one page, the routine effectively
> becomes a boolean. Hence the name "needs".
>
> vma_commit_reservation - This is a wrapper for region_add(). It
> must be called after a prior call to vma_needs_reservation
> and after actual allocation of the page.
>
> We need a way to handle the case where vma_needs_reservation has
> been called, but the page allocation is not successful. I chose
> the name vma_abort_reservation, but as noted (even in my comments)
> it is not an actual abort.
>
> I am not sure if you are suggesting vma_commit_reservation() should
> handle this as a special case. I think a separately named routine which
> indicates and end of the reservation/allocation process would be
> easier to understand.
>
> What about changing the name vma_abort_reservation() to
> vma_end_reservation()? This would indicate that the reservation/
> allocation process is ended.
OK, vma_end_reservation() sounds nice to me.
> > Thanks,
> > Naoya Horiguchi
>
> Thank you for your reviews.
You're welcome :)
Naoya Horiguchi--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
