All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefani Seibold <stefani@seibold.net>
To: Arnd Bergmann <arnd@arndb.de>
Cc: linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [RFC 0/2] new kfifo API
Date: Tue, 04 Aug 2009 14:44:37 +0200	[thread overview]
Message-ID: <1249389877.11474.14.camel@wall-e> (raw)
In-Reply-To: <200908041424.54472.arnd@arndb.de>

Am Dienstag, den 04.08.2009, 14:24 +0200 schrieb Arnd Bergmann:
> On Monday 03 August 2009, Stefani Seibold wrote:
> > Am Montag, den 03.08.2009, 21:00 +0200 schrieb Arnd Bergmann:
> > 
> > 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....
> 
> Yes, you are right. I realized that myself after I sent out my
> comments.
> 
> > > 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?
> 
> vmalloc also has performance problems on some architectures that can
> access the linear mapping faster than paged memory and it is
> rather wasteful if you have 64kb pages.
> 
> I don't think the total size matters, the 128 MB limit only exists
> if you have a 32 bit CPU and 1GB or more of memory, which is hopefully
> getting rarer and already causes other problems (highmem...).
> 
> kmalloc currently limits the kfifo size to something like 128kb (arch
> specific), if you need more than that, you need alloc_pages(), which
> is limited to a power-of-two amount of pages.
> 

You are right, i don't like vmalloc too. It was only a first thought ;-)

> > > 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.
> 
> I think this is actually more relevant than the vmalloc limit you mentioned,
> demand for tiny processors will probably stay because of cost reasons.
> Architectures that we support in Linux without integer divide include
> arm, blackfin, h8300, ia64 (!), m68k, microblaze, sh and xtensa.
> 

I'm embedded developer too... so i know what you mean. But for the fifo
this is not really a problem, managing and copying the data will be the
bigger amount. But with my new version i go back to the old power of two
method.

> Your first version with the non-power-of-two buffers also had a bug
> in the handling because it would not handle 32 bit integer overflows
> correctly. To get those right, you need an extra branch every time you
> add to the counter.
> 

Ooops, that was a real bug... Thanks.

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

> 
> 	Arnd <><

Stefani <\_,



  reply	other threads:[~2009-08-04 12:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-03 13:39 [RFC 0/2] new kfifo API Stefani Seibold
2009-08-03 14:42 ` Arnd Bergmann
2009-08-03 15:14   ` Stefani Seibold
2009-08-03 18:23     ` Arnd Bergmann
2009-08-03 18:45       ` Stefani Seibold
2009-08-03 16:41   ` Mike Christie
2009-08-03 18:27 ` Andi Kleen
2009-08-03 18:35   ` Arnd Bergmann
2009-08-03 18:48   ` Stefani Seibold
2009-08-03 19:00 ` Arnd Bergmann
2009-08-03 19:48   ` Stefani Seibold
2009-08-04 12:24     ` Arnd Bergmann
2009-08-04 12:44       ` Stefani Seibold [this message]
2009-08-04 13:45         ` Arnd Bergmann
2009-08-04 14:57           ` Stefani Seibold
2009-08-04 18:00             ` Arnd Bergmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1249389877.11474.14.camel@wall-e \
    --to=stefani@seibold.net \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.