linux-kernel - Re: [PATCH -mm 1/6] slab: introduce __kfree

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: <alpine.DEB.1.10.0903231234110.11796@qirst.com>
Date:	Mon, 23 Mar 2009 12:40:14 -0400 (EDT)
From:	Christoph Lameter <cl@...ux-foundation.org>
To:	Ingo Molnar <mingo@...e.hu>
cc:	Pekka Enberg <penberg@...helsinki.fi>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Nick Piggin <npiggin@...e.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Manfred Spraul <manfred@...orfullife.com>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH -mm 1/6] slab: introduce __kfree_rcu

On Mon, 23 Mar 2009, Ingo Molnar wrote:

>
> * Christoph Lameter <cl@...ux-foundation.org> wrote:
>
> > On Mon, 23 Mar 2009, Pekka Enberg wrote:
> >
> > > > +static inline void *portion_to_obj(void *portion)
> > > > +{
> > > > +	struct page *page = virt_to_head_page(portion);
> > > > +	struct slab *slab = page_get_slab(page);
> > > > +	struct kmem_cache *cache = page_get_cache(page);
> > > > +	unsigned int offset = portion - slab->s_mem;
> > > > +	unsigned int index = offset / cache->buffer_size;
> > > > +
> > > > +	return index_to_obj(cache, slab, index);
> > > > +}
> > >
> > > A minor nit: I think this would be more readable if you separated
> > > variable declarations from the initializations. Also, you can probably
> > > drop the inline from the function declaration and let GCC decide what to
> > > do.
> >
> > Thats debatable. I find the setting up a number of variables that
> > are all dependend in the above manner very readable. They are
> > usually repetitive. Multiple functions use similar
> > initializations.
>
> I agree with Pekka, it's clearly more readable when separated out
> nicely:
>
> 	struct kmem_cache *cache;
> 	unsigned int offset;
> 	unsigned int index;
> 	struct page *page;
> 	struct slab *slab;
>
> 	page	= virt_to_head_page(portion);
> 	slab	= page_get_slab(page);
> 	cache	= page_get_cache(page);
>
> 	offset	= portion - slab->s_mem;
> 	index	= offset / cache->buffer_size;
>
> The original form is hard to read due to lack of structure.

Structure can also be established differently:

static inline void *portion_to_obj(void *portion)
{
	struct page *page = virt_to_head_page(portion);
	struct slab *slab = page_get_slab(page);
	struct kmem_cache *cache = page_get_cache(page);

	unsigned int offset = portion - slab->s_mem;
	unsigned int index = offset / cache->buffer_size;

	return index_to_obj(cache, slab, index);
}

It would be good if the whole series of actions that need to be taken in
order for the function to "get to know" the slab the object parms would be
simpler. Like its done in ruby

	(page, slab, cache) = get_slab_info(portion)

	(offset, index) = get_position_info(slab, portion)

But how can this be done in C without weird pointer passing?

--
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/