Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751577AbbGUESo (ORCPT ); Tue, 21 Jul 2015 00:18:44 -0400 Received: from TYO201.gate.nec.co.jp ([210.143.35.51]:54709 "EHLO tyo201.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750771AbbGUESn convert rfc822-to-8bit (ORCPT ); Tue, 21 Jul 2015 00:18:43 -0400 From: Naoya Horiguchi To: Mike Kravetz CC: "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "linux-api@vger.kernel.org" , Dave Hansen , "David Rientjes" , Hugh Dickins , "Davidlohr Bueso" , Aneesh Kumar , Hillf Danton , Christoph Hellwig , Andrew Morton , Michal Hocko Subject: Re: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add Thread-Topic: [PATCH v3 01/10] mm/hugetlb: add cache of descriptors to resv_map for region_add Thread-Index: AQHQvSNnIgScQR2NukuUNK/TwtEgrZ3ezWUAgAVKgQCAAK78gA== Date: Tue, 21 Jul 2015 04:16:30 +0000 Message-ID: <20150721041629.GA19982@hori1.linux.bs1.fc.nec.co.jp> References: <1436761268-6397-1-git-send-email-mike.kravetz@oracle.com> <1436761268-6397-2-git-send-email-mike.kravetz@oracle.com> <20150717090213.GB32135@hori1.linux.bs1.fc.nec.co.jp> <55AD34D4.2020804@oracle.com> In-Reply-To: <55AD34D4.2020804@oracle.com> Accept-Language: ja-JP, en-US Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.128.101.29] Content-Type: text/plain; charset="iso-2022-jp" Content-ID: <22AD3C911DA5AE4BAF7FA181F0F5C9B4@gisp.nec.co.jp> Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2492 Lines: 63 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/