Message-Id: <1249328898.5106.44.camel@wall-e>
Date:	Mon, 03 Aug 2009 21:48:18 +0200
From:	Stefani Seibold <stefani@...bold.net>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC 0/2] new kfifo API

Am Montag, den 03.08.2009, 21:00 +0200 schrieb Arnd Bergmann:
> Going through your list again:
> 
> On Monday 03 August 2009, Stefani Seibold wrote:
> > - Generic usage: For kernel internal use or device driver
> 
> no change here, right?
> 
> > - Linux style habit: DECLARE_KFIFO, DEFINE_KFIFO and INIT_KFIFO Macros

> DEFINE_KFIFO looks useful, but I probably wouldn't expose
> the other macros, so you could define them as __KFIFO_* or
> integrate them into a larger DEFINE_KFIFO.
> 

DECLARE_KFIFO looks for me more useful, because i can use it inside a
struct decalaration. And then i need INIT_KFIFO for initializing this.

BTW: DECLARE_...., DEFINE_..... and INIT_..... are linux style. Habe a
look at workqueue.h, wait.h, types.h, semaphore.h, rwsem-spinlock.h,
interrupt.h, completion.h, seqlock.h and so on....

> > - Ability to handle variable length records. Three type of records are
> >   supported:
> >   - Records between 0-255 bytes, with a record size field of 1 bytes
> >   - Records between 0-65535 bytes, with a record size field of 2 bytes
> >   - Byte stream, which no record size field
> >   Inside a fifo this record types it is not a good idea to mix them together.
> 
> Not sure if having both 1 and 2 byte record lengths really helps.
> If you want to avoid mixing the two, maybe just leave the existing
> API for byte streams in a compatible, and provide extra functions
> for records with a definite length.
> 

Streams are only a specially case of a very huge record. I personally
never needed records greater than 65535 byte. But i is easy to extend it
to support 3 and 4 byte records length field too. 

And mixing different record size fields makes no sense. If you know that
your records can be create than 255 bytes then use a 2 byte record
field. It is only the recsize parameter, which can be 0, 1 or 2.

0 means byte stream mode, 1 means records size from 0 to 255, and 2
means records size from 0 to 65535. 

It is designed like any other container or lists in the linux kernel.
The developer must know what she is doing ;-) Error checking wastes cpu
rescources.

> > - Direct copy_to_user from the fifo and copy_from_user into the fifo.
> 
> Sounds useful, as mentioned.
> 
> > - Single structure: The fifo structure contains the management variables and
> >   the buffer. No extra indirection is needed to access the fifo buffer.
> 
> I see two problems here:
> 
> 1. you can no longer use preallocated buffers, which limits the possible
> users to those that are unrestricted to the type of allocation.
> 2. The size of the buffer is no longer power-of-two. In fact, it's guaranteed
> to be non-power-of-two because kmalloc gives you a power-of-two allocation
> but now you also put the struct kfifo in there.
> 
> Users that need a power-of-two buffer (the common case) now waste almost
> 50% of the space.
> 

Okay, give me a thought about this....... yes you are right ;-( But what
is with vmalloc? 128 MB should be enough?

> The requirement for power-of-two also meant a much faster __kfifo_off
> function on certain embedded platforms that don't have an integer division
> instruction in hardware.
> 

Yes i know this argument, but since the day of the 6502 and Z80 i have
never seen this kind of CPU. Okay i forgot to mention the stupid ARM
CPU, but newer ARM cores have a hardware division support.

Stefani <\_,
         ^ ^


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

Powered by blists - more mailing lists