From owner-svn-src-head@FreeBSD.ORG Thu Jun 12 23:46:21 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0485E4B2; Thu, 12 Jun 2014 23:46:21 +0000 (UTC) Received: from mail-qc0-x22d.google.com (mail-qc0-x22d.google.com [IPv6:2607:f8b0:400d:c01::22d]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 892D326FC; Thu, 12 Jun 2014 23:46:20 +0000 (UTC) Received: by mail-qc0-f173.google.com with SMTP id l6so3123028qcy.4 for ; Thu, 12 Jun 2014 16:46:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=ObForiqJSK0fY7TbAMRN5+QBd1/zTPh8IIDeePriojQ=; b=tG3NK5paur0npXFQ6iKftHKZhyH13SWTrN4IkXflcqon/uVuUZ6EdQH2Hp3JOmaDoc VLY1KGHoU+qM5RlguXFJ+/ZWMXJj9mkxCvAVikq1fFpyFjybbVt6v919Qerntm2iyy0d wdN1ViSdEyulH/GiTqf50BJrmzg6x8h2AKLYqJf/KmmTbOi3qfBTfDZhw/I7VHz/UMdn GRghLBpjt/osDuHmNNmpf5v1zGwklfU6iTz5TUuBIf3XrO4wRPM+fz3kdI2nEwALt/wu wdh6OjeXLwcFixAFruL5O/jqRbVZxP2PAmrNUZ3JZbDicnbbqZ/uysWTfmQfDLv2hRPd LMxQ== MIME-Version: 1.0 X-Received: by 10.224.130.136 with SMTP id t8mr68671118qas.49.1402616778980; Thu, 12 Jun 2014 16:46:18 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.224.43.134 with HTTP; Thu, 12 Jun 2014 16:46:18 -0700 (PDT) In-Reply-To: <201406111611.s5BGBCHU058618@svn.freebsd.org> References: <201406111611.s5BGBCHU058618@svn.freebsd.org> Date: Thu, 12 Jun 2014 18:46:18 -0500 X-Google-Sender-Auth: uHTCQuerV81Awq7hX1G4rBZx9Xs Message-ID: Subject: Re: svn commit: r267364 - head/sys/vm From: Adrian Chadd To: Alan Cox Content-Type: text/plain; charset=UTF-8 Cc: "svn-src-head@freebsd.org" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jun 2014 23:46:21 -0000 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? -a 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; >