Re: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add
From: Naoya Horiguchi <hidden>
Date: 2015-07-21 04:18:44
Also in:
linux-mm, lkml
On Mon, Jul 20, 2015 at 10:50:12AM -0700, Mike Kravetz wrote: ...
quoted
...quoted
@@ -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.
quoted
Thanks, Naoya HoriguchiThank you for your reviews.
You're welcome :) Naoya Horiguchi -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>