linux-kernel - Re: [RFC 0/2] new kfifo API

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: <200908041545.37329.arnd@arndb.de>
Date:	Tue, 4 Aug 2009 15:45:37 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Stefani Seibold <stefani@...bold.net>
Cc:	"linux-kernel" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 0/2] new kfifo API

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/