From owner-svn-src-all@FreeBSD.ORG Fri Jun 13 06:44:33 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A0C0E3B9; Fri, 13 Jun 2014 06:44:33 +0000 (UTC) Received: from pp2.rice.edu (proofpoint2.mail.rice.edu [128.42.201.101]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6479A2764; Fri, 13 Jun 2014 06:44:32 +0000 (UTC) Received: from pps.filterd (pp2.rice.edu [127.0.0.1]) by pp2.rice.edu (8.14.5/8.14.5) with SMTP id s5D6ekGL019766; Fri, 13 Jun 2014 01:44:30 -0500 Received: from mh1.mail.rice.edu (mh1.mail.rice.edu [128.42.201.20]) by pp2.rice.edu with ESMTP id 1mfh28g805-1; Fri, 13 Jun 2014 01:44:29 -0500 X-Virus-Scanned: by amavis-2.7.0 at mh1.mail.rice.edu, auth channel Received: from 108-254-203-201.lightspeed.hstntx.sbcglobal.net (108-254-203-201.lightspeed.hstntx.sbcglobal.net [108.254.203.201]) (using TLSv1 with cipher RC4-MD5 (128/128 bits)) (No client certificate requested) (Authenticated sender: alc) by mh1.mail.rice.edu (Postfix) with ESMTPSA id F209B460196; Fri, 13 Jun 2014 01:44:28 -0500 (CDT) Message-ID: <539A9DCB.20205@rice.edu> Date: Fri, 13 Jun 2014 01:44:27 -0500 From: Alan Cox User-Agent: Mozilla/5.0 (X11; FreeBSD i386; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Adrian Chadd , Alan Cox Subject: Re: svn commit: r267364 - head/sys/vm References: <201406111611.s5BGBCHU058618@svn.freebsd.org> In-Reply-To: X-Enigmail-Version: 1.6 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 kscore.is_bulkscore=0 kscore.compositescore=0 circleOfTrustscore=0 compositescore=0.765433954564634 urlsuspect_oldscore=0.765433954564634 suspectscore=13 recipient_domain_to_sender_totalscore=0 phishscore=0 bulkscore=0 kscore.is_spamscore=1 recipient_to_sender_totalscore=0 recipient_domain_to_sender_domain_totalscore=0 rbsscore=0.765433954564634 spamscore=0 recipient_to_sender_domain_totalscore=0 urlsuspectscore=0.9 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=7.0.1-1402240000 definitions=main-1406130086 Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jun 2014 06:44:33 -0000 On 06/12/2014 18:46, Adrian Chadd wrote: > Hi! > > Wouldn't this also be fixed if popmap_t was a unsigned char (to match > what setbit/clrbit/etc do) and then use the existing macros? > > Is there any reason popmap_t is not a uchar ? Is it ever accessed as > anything other than an array of chars? Yes, there is. Parts of the code that you don't see in the below change make heavy use of ffsl() on the population map. > > > On 11 June 2014 11:11, Alan Cox wrote: >> Author: alc >> Date: Wed Jun 11 16:11:12 2014 >> New Revision: 267364 >> URL: http://svnweb.freebsd.org/changeset/base/267364 >> >> Log: >> Correct a bug in the management of the population map on big-endian >> machines. Specifically, there was a mismatch between how the routine >> allocation and deallocation operations accessed the population map >> and how the aggressively optimized reservation-breaking operation >> accessed it. So, problems only occurred when reservations were broken. >> This change makes the routine operations access the population map in >> the same way as the reservation breaking operation. >> >> This bug was introduced in r259999. >> >> PR: 187080 >> Tested by: jmg (on an "armeb" machine) >> Sponsored by: EMC / Isilon Storage Division >> >> Modified: >> head/sys/vm/vm_reserv.c >> >> Modified: head/sys/vm/vm_reserv.c >> ============================================================================== >> --- head/sys/vm/vm_reserv.c Wed Jun 11 14:53:58 2014 (r267363) >> +++ head/sys/vm/vm_reserv.c Wed Jun 11 16:11:12 2014 (r267364) >> @@ -108,6 +108,46 @@ typedef u_long popmap_t; >> #define NPOPMAP howmany(VM_LEVEL_0_NPAGES, NBPOPMAP) >> >> /* >> + * Clear a bit in the population map. >> + */ >> +static __inline void >> +popmap_clear(popmap_t popmap[], int i) >> +{ >> + >> + popmap[i / NBPOPMAP] &= ~(1UL << (i % NBPOPMAP)); >> +} >> + >> +/* >> + * Set a bit in the population map. >> + */ >> +static __inline void >> +popmap_set(popmap_t popmap[], int i) >> +{ >> + >> + popmap[i / NBPOPMAP] |= 1UL << (i % NBPOPMAP); >> +} >> + >> +/* >> + * Is a bit in the population map clear? >> + */ >> +static __inline boolean_t >> +popmap_is_clear(popmap_t popmap[], int i) >> +{ >> + >> + return ((popmap[i / NBPOPMAP] & (1UL << (i % NBPOPMAP))) == 0); >> +} >> + >> +/* >> + * Is a bit in the population map set? >> + */ >> +static __inline boolean_t >> +popmap_is_set(popmap_t popmap[], int i) >> +{ >> + >> + return ((popmap[i / NBPOPMAP] & (1UL << (i % NBPOPMAP))) != 0); >> +} >> + >> +/* >> * The reservation structure >> * >> * A reservation structure is constructed whenever a large physical page is >> @@ -241,7 +281,7 @@ vm_reserv_depopulate(vm_reserv_t rv, int >> mtx_assert(&vm_page_queue_free_mtx, MA_OWNED); >> KASSERT(rv->object != NULL, >> ("vm_reserv_depopulate: reserv %p is free", rv)); >> - KASSERT(isset(rv->popmap, index), >> + KASSERT(popmap_is_set(rv->popmap, index), >> ("vm_reserv_depopulate: reserv %p's popmap[%d] is clear", rv, >> index)); >> KASSERT(rv->popcnt > 0, >> @@ -255,7 +295,7 @@ vm_reserv_depopulate(vm_reserv_t rv, int >> rv)); >> rv->pages->psind = 0; >> } >> - clrbit(rv->popmap, index); >> + popmap_clear(rv->popmap, index); >> rv->popcnt--; >> if (rv->popcnt == 0) { >> LIST_REMOVE(rv, objq); >> @@ -302,7 +342,7 @@ vm_reserv_populate(vm_reserv_t rv, int i >> mtx_assert(&vm_page_queue_free_mtx, MA_OWNED); >> KASSERT(rv->object != NULL, >> ("vm_reserv_populate: reserv %p is free", rv)); >> - KASSERT(isclr(rv->popmap, index), >> + KASSERT(popmap_is_clear(rv->popmap, index), >> ("vm_reserv_populate: reserv %p's popmap[%d] is set", rv, >> index)); >> KASSERT(rv->popcnt < VM_LEVEL_0_NPAGES, >> @@ -313,7 +353,7 @@ vm_reserv_populate(vm_reserv_t rv, int i >> TAILQ_REMOVE(&vm_rvq_partpop, rv, partpopq); >> rv->inpartpopq = FALSE; >> } >> - setbit(rv->popmap, index); >> + popmap_set(rv->popmap, index); >> rv->popcnt++; >> if (rv->popcnt < VM_LEVEL_0_NPAGES) { >> rv->inpartpopq = TRUE; >> @@ -503,7 +543,7 @@ found: >> return (NULL); >> /* Handle vm_page_rename(m, new_object, ...). */ >> for (i = 0; i < npages; i++) >> - if (isset(rv->popmap, index + i)) >> + if (popmap_is_set(rv->popmap, index + i)) >> return (NULL); >> for (i = 0; i < npages; i++) >> vm_reserv_populate(rv, index + i); >> @@ -628,7 +668,7 @@ found: >> index = VM_RESERV_INDEX(object, pindex); >> m = &rv->pages[index]; >> /* Handle vm_page_rename(m, new_object, ...). */ >> - if (isset(rv->popmap, index)) >> + if (popmap_is_set(rv->popmap, index)) >> return (NULL); >> vm_reserv_populate(rv, index); >> return (m); >> @@ -662,9 +702,9 @@ vm_reserv_break(vm_reserv_t rv, vm_page_ >> * to the physical memory allocator. >> */ >> i = m - rv->pages; >> - KASSERT(isclr(rv->popmap, i), >> + KASSERT(popmap_is_clear(rv->popmap, i), >> ("vm_reserv_break: reserv %p's popmap is corrupted", rv)); >> - setbit(rv->popmap, i); >> + popmap_set(rv->popmap, i); >> rv->popcnt++; >> } >> i = hi = 0; >> >