From: Stefani Seibold <stefani@seibold.net>
To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] kfifo: round up the fifo size power of 2
Date: Wed, 14 Nov 2012 08:03:49 +0100 [thread overview]
Message-ID: <1352876629.7469.3.camel@wall-e> (raw)
In-Reply-To: <20121109023221.GO18293@yliu-dev.sh.intel.com>
Am Freitag, den 09.11.2012, 10:32 +0800 schrieb Yuanhan Liu:
> On Thu, Nov 08, 2012 at 01:37:15PM +0100, Stefani Seibold wrote:
> > Am Donnerstag, den 08.11.2012, 20:24 +0800 schrieb Yuanhan Liu:
> Yes, it is. I will try log API then.
>
> Stefani, I found an issue while rework to current API. Say the current
> code of __kfifo_init:
> int __kfifo_init(struct __kfifo *fifo, void *buffer,
> unsigned int size, size_t esize)
> {
> size /= esize;
>
> if (!is_power_of_2(size))
> size = rounddown_pow_of_two(size);
> ....
> }
>
> Even thought I changed the API to something like:
> int __kfifo_init(struct __kfifo *fifo, void *buffer,
> int size_order, size_t esize)
> {
> unsigned int size = 1 << size_order;
>
> size /= esize;
> ...
> }
>
> See? There is still a divide and we can't make it sure that it will be
> power of 2 after that.
>
> So, I came up 2 proposal to fix this.
>
> 1. refactor the meaning of 'size' argument first.
>
> 'size' means the size of pre-allocated buffer. We can refactor it to
> meaning of 'the number of fifo elements' just like __kfifo_alloc, so
> that we don't need do the size /= esize stuff.
>
> 2. remove kfifo_init
>
> As we can't make sure that kfifo will do exactly what users asked(in
> the way of fifo size). It would be safe and good to maintain buffer
> and buffer size inside kfifo. So, I propose to remove it and use
> kfifo_alloc instead.
>
> git grep 'kfifo_init\>' shows that we currently have 2 users only.
>
>
> The first way is hacky, and it doesn't make much sense to me. Since
> buffer is pre-allocated by user but not kfifo. User has to calculate
> element size and the number of elements, which is not friendly.
>
> The second way does make more sense to me.
kfifo_init() was requested by some kernel developers, i never liked it.
If you have a better and cleaner solution than do it, otherwise kick it
away if you like.
- Stefani
next prev parent reply other threads:[~2012-11-14 7:04 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-26 7:56 [PATCH 1/2] kfifo: round up the fifo size power of 2 Yuanhan Liu
2012-10-26 7:56 ` [PATCH 2/2] kfifo: handle the case that alloc size is equal to 0 Yuanhan Liu
2012-10-26 9:30 ` [PATCH 1/2] kfifo: round up the fifo size power of 2 Stefani Seibold
2012-10-26 12:33 ` Yuanhan Liu
2012-10-26 13:39 ` Stefani Seibold
2012-10-26 14:06 ` Yuanhan Liu
2012-10-26 14:23 ` Alan Cox
2012-10-29 20:59 ` Andrew Morton
2012-10-31 5:59 ` Yuanhan Liu
2012-10-31 6:30 ` Stefani Seibold
2012-10-31 6:49 ` Yuanhan Liu
2012-10-31 20:31 ` Stefani Seibold
2012-10-31 6:52 ` Andrew Morton
2012-10-31 8:11 ` Janne Kulmala
2012-10-31 11:16 ` Andrew Morton
2012-10-31 20:31 ` Stefani Seibold
2012-11-08 12:24 ` Yuanhan Liu
2012-11-08 12:37 ` Stefani Seibold
2012-11-09 2:32 ` Yuanhan Liu
2012-11-14 7:03 ` Stefani Seibold [this message]
2012-11-15 8:18 ` Yuanhan Liu
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=1352876629.7469.3.camel@wall-e \
--to=stefani@seibold.net \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=yuanhan.liu@linux.intel.com \
/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.