linux-kernel - Re: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to resv_map for region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150721041629.GA19982@hori1.linux.bs1.fc.nec.co.jp>
Date:	Tue, 21 Jul 2015 04:16:30 +0000
From:	Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To:	Mike Kravetz <mike.kravetz@...cle.com>
CC:	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	"David Rientjes" <rientjes@...gle.com>,
	Hugh Dickins <hughd@...gle.com>,
	"Davidlohr Bueso" <dave@...olabs.net>,
	Aneesh Kumar <aneesh.kumar@...ux.vnet.ibm.com>,
	Hillf Danton <hillf.zj@...baba-inc.com>,
	Christoph Hellwig <hch@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Hocko <mhocko@...e.cz>
Subject: Re: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to
 resv_map for region_add

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/