linux-kernel - Re: [RFCv2][PATCH] flexible array implementation

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]

Date:	Tue, 21 Jul 2009 21:34:52 -0700
From:	Dave Hansen <dave@...ux.vnet.ibm.com>
To:	Li Zefan <lizf@...fujitsu.com>
Cc:	akpm@...ux-foundation.org, containers@...ts.linux-foundation.org,
	bblum@...gle.com, linux-kernel@...r.kernel.org, menage@...gle.com,
	vda.linux@...glemail.com, mikew@...gle.com
Subject: Re: [RFCv2][PATCH] flexible array implementation

On Wed, 2009-07-22 at 11:25 +0800, Li Zefan wrote:
> > +/**
> > + * flex_array_put - copy data into the array at @element_nr
> > + * @src:	address of data to copy into the array
> > + * @element_nr:	index of the position in which to insert
> > + * 		the new element.
> 
> @fa and @flags are not documented.

True...  But one of my pet peeves are kerneldocs like this:

	@fa: the flex array
	@flags: GFP flags

It's so trivially obvious from looking at the types and the variable
names that I'm not sure it's worth the cost of the lines.  

> > + *
> > + * Note that this *copies* the contents of @src into
> > + * the array.  If you are trying to store an array of
> > + * pointers, make sure to pass in &ptr instead of ptr.
> > + *
> > + * Locking must be provided by the caller.
> > + */
> > +int flex_array_put(struct flex_array *fa, int element_nr, void *src, gfp_t flags)
> > +{
> > +	int part_nr = fa_element_to_part_nr(fa, element_nr);
> > +	struct flex_array_part *part;
> > +	void *dst;
> > +
> > +	part = __fa_get_part(fa, part_nr, flags);
> > +	if (!part)
> > +		return -ENOMEM;
> 
> So this may allocate memory, and has disavantages:
> 
> - If flex_array_put() is called in atomic context, flags has to be GFP_ATOMIC.
> - and thus it may fail.
> 
> Since we pass the total_elem to flex_array_alloc(), how about add a flag,
> and if the flag is set, the alloc() will also allocate all fa_parts?
> 
> And add __flex_array_put(), which assumes fa_parts has been allocated.

How about flex_array_prealloc()?  It seems to work for all the radix
tree users.

-- Dave

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