linux-kernel - Re: [PATCH 4/7] zram/xvmalloc: free bit block insertion optimization

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110126184057.GC17383@linux.vnet.ibm.com>
Date:	Wed, 26 Jan 2011 12:40:57 -0600
From:	Robert Jennings <rcj@...ux.vnet.ibm.com>
To:	Pekka Enberg <penberg@...nel.org>
Cc:	Nitin Gupta <ngupta@...are.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Robert Jennings <rcj@...ux.vnet.ibm.com>,
	Pekka Enberg <penberg@...helsinki.fi>,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/7] zram/xvmalloc: free bit block insertion
 optimization

* Pekka Enberg (penberg@...nel.org) wrote:
> On Wed, Jan 26, 2011 at 7:25 PM, Robert Jennings <rcj@...ux.vnet.ibm.com> wrote:
> > There is no need to set the bits in the first- and second-level indices
> > to indicate a free page when we know that a free page existed at this
> > level.
> >
> > Signed-off-by: Robert Jennings <rcj@...ux.vnet.ibm.com>
> 
> Why is it not necessary? I don't know that part of the code well
> enough to tell if this patch is safe or not.

This change is in a conditional block which is entered only when there is
an existing data block on the freelist where the insert has taken place.

The new block is pushed onto the freelist stack and this conditional block
is updating links in the prior stack head to point to the new stack head.
After this conditional block the first-/second-level indices are updated
to indicate that there is a free block at this location.

This patch adds an immediate return from the conditional block to avoid
setting bits again to indicate a free block on this freelist. They would
already be set because there was an existing free block on this freelist.

> > ---
> >  drivers/staging/zram/xvmalloc.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/staging/zram/xvmalloc.c b/drivers/staging/zram/xvmalloc.c
> > index a507f95..b3622f1 100644
> > --- a/drivers/staging/zram/xvmalloc.c
> > +++ b/drivers/staging/zram/xvmalloc.c
> > @@ -205,6 +205,8 @@ static void insert_block(struct xv_pool *pool, struct page *page, u32 offset,
> >                nextblock->link.prev_page = page;
> >                nextblock->link.prev_offset = offset;
> >                put_ptr_atomic(nextblock, KM_USER1);
> > +               /* If there was a next page then the free bits are set. */
> > +               return;
> >        }
> >
> >        __set_bit(slindex % BITS_PER_LONG, &pool->slbitmap[flindex]);
--
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/