From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755695AbbEUXnm (ORCPT ); Thu, 21 May 2015 19:43:42 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:35670 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753397AbbEUXnk (ORCPT ); Thu, 21 May 2015 19:43:40 -0400 Message-ID: <555E6D99.7070200@oracle.com> Date: Thu, 21 May 2015 16:43:21 -0700 From: Mike Kravetz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Andrew Morton CC: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Naoya Horiguchi , Davidlohr Bueso , David Rientjes , Luiz Capitulino Subject: Re: [PATCH 1/2] mm/hugetlb: compute/return the number of regions added by region_add() References: <1431971349-6668-1-git-send-email-mike.kravetz@oracle.com> <1431971349-6668-2-git-send-email-mike.kravetz@oracle.com> <20150521163027.566f3f0fd82801f2140a420d@linux-foundation.org> In-Reply-To: <20150521163027.566f3f0fd82801f2140a420d@linux-foundation.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: aserv0022.oracle.com [141.146.126.234] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2015 04:30 PM, Andrew Morton wrote: > On Mon, 18 May 2015 10:49:08 -0700 Mike Kravetz 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? Will do. I'll provide an updated patch to address your questions and better document this whole region/reserve map stuff. It took me a few days to wrap my head around how it actually works. -- Mike Kravetz > - One has to do an extraordinary amount of reading to discover that > the units of file_region.from and .to are "multiples of > 1< > 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. >