Re: [RFC 0/2] new kfifo API

[Arnd Bergmann]

On Tuesday 04 August 2009, Stefani Seibold wrote:
> > Your second version is ok in this regard because it uses the original
> > size logic.
> 
> Does it mean you like it now ;-) ???? I think we are on a good way!

It looks much better now, but I still think you are doing too many
things at once, and I disagree about the locking changes.

I think it would be best to have an incremental set of patches
to the original code, along the lines of

[PATCH 1/x] kfifo: preparation code reorg, no functional change
[PATCH 2/x] kfifo: add DEFINE_KFIFO and friends
[PATCH 3/x] kfifo: add kfifo_{to,from}_user
[PATCH 4/x] kfifo: add kfifo_{get,put}_rec
[PATCH 5/x] kfifo: ...

About the locking stuff, I think it should best be left in place.
The __kfifo_{get,put} functions should probably be declared part
of the official interface and documented as such -- people are
using them anyways. It's generally a good idea to have the obvious
interface work in an entirely safe way (kfifo_get doing all the
locking it might need), with a __foo variant of the same function
for people that want the extra performance and know what they are
doing.

I would also leave out the recsize argument, using an 'unsigned short'
for the record length unconditionally won't waste any real space but
simplifies both the implementation and the interface.

Finally, I don't see a reason for the optional KFIFO_F_NOTRIM argument.
If you have fixed records, I would guess that you always need it
anyway, so you could just make it the default and remove the function
argument.

	Arnd <><
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/