From: Stefani Seibold <stefani@seibold.net>
To: Andi Kleen <andi@firstfloor.org>
Cc: linux-kernel@vger.kernel.org, akpm@osdl.org
Subject: Re: [PATCH] [2/6] kfifo: Make kfifo_in atomic
Date: Sun, 27 Dec 2009 22:46:45 +0100 [thread overview]
Message-ID: <1261950405.25298.30.camel@wall-e> (raw)
In-Reply-To: <20091227210312.A60FCB17C3@basil.firstfloor.org>
Am Sonntag, den 27.12.2009, 22:03 +0100 schrieb Andi Kleen:
> Right now kfifo_in allows copying in less than the input
> amount. This is unfortunately not a good idea on any
> record oriented users: if the size of the kfifo is not
> a multiple of the record (and that can easily happen due
> to the power-of-two requirement) then when the FIFO fills
> up partial records could be put in. Such a condition would
> be fatal for any record consumer who would get permanently
> desynchronized. In fact I doubt unless the input
> is a totally boundary less data stream I doubt anything
> could handle this.
>
> Change kfifo_in() to always put in everything or nothing.
>
> The return value is now always 0 or the full length.
>
That is why had written the macro version of kfifo. This provide an type
save kfifo implementation where struct records never will be splitted.
Have a look at the sample code in
http://patchwork.kernel.org/patch/69093/
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> ---
> kernel/kfifo.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> Index: linux/kernel/kfifo.c
> ===================================================================
> --- linux.orig/kernel/kfifo.c
> +++ linux/kernel/kfifo.c
> @@ -228,9 +228,8 @@ EXPORT_SYMBOL(__kfifo_in_n);
> * @from: the data to be added.
> * @len: the length of the data to be added.
> *
> - * This function copies at most @len bytes from the @from buffer into
> - * the FIFO depending on the free space, and returns the number of
> - * bytes copied.
> + * This function copies @len bytes from the @from buffer into
> + * the FIFO and returns 0 if there is not enough space.
> *
> * Note that with only one concurrent reader and one concurrent
> * writer, you don't need extra locking to use these functions.
> @@ -238,8 +237,8 @@ EXPORT_SYMBOL(__kfifo_in_n);
> unsigned int kfifo_in(struct kfifo *fifo, const void *from,
> unsigned int len)
> {
> - len = min(kfifo_avail(fifo), len);
> -
> + if (kfifo_avail(fifo) < len)
> + return 0;
> __kfifo_in_data(fifo, from, len, 0);
> __kfifo_add_in(fifo, len);
> return len;
next prev parent reply other threads:[~2009-12-27 21:46 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-27 21:03 [PATCH] [0/6] kfifo fixes/improvements Andi Kleen
2009-12-27 21:03 ` [PATCH] [1/6] kfifo: Use void * pointers for user buffers Andi Kleen
2009-12-27 21:48 ` Stefani Seibold
2009-12-27 21:03 ` [PATCH] [2/6] kfifo: Make kfifo_in atomic Andi Kleen
2009-12-27 21:46 ` Stefani Seibold [this message]
2009-12-27 21:03 ` [PATCH] [3/6] kfifo: Sanitize *_user error handling Andi Kleen
2009-12-27 21:38 ` Stefani Seibold
2009-12-27 23:34 ` Andi Kleen
2009-12-28 7:10 ` Stefani Seibold
2010-01-04 22:33 ` Stefani Seibold
2009-12-27 21:03 ` [PATCH] [4/6] kfifo: add kfifo_out_peek Andi Kleen
2009-12-27 21:49 ` Stefani Seibold
2009-12-27 23:41 ` Andi Kleen
2009-12-28 7:09 ` Stefani Seibold
2010-01-04 21:57 ` Andrew Morton
2010-01-04 22:24 ` Alan Cox
2010-01-04 22:47 ` Stefani Seibold
2010-01-05 0:14 ` Alan Cox
2009-12-27 21:03 ` [PATCH] [5/6] kfifo: Add kfifo_initialized Andi Kleen
2009-12-27 21:53 ` Stefani Seibold
2009-12-27 21:03 ` [PATCH] [6/6] kfifo: Document everywhere that size has to be power of two Andi Kleen
2009-12-27 21:50 ` Stefani Seibold
2009-12-27 22:14 ` Dmitry Torokhov
2009-12-27 22:23 ` Stefani Seibold
2009-12-27 23:34 ` Andi Kleen
2009-12-27 21:36 ` [PATCH] [0/6] kfifo fixes/improvements Stefani Seibold
2009-12-27 23:38 ` Andi Kleen
2009-12-28 6:49 ` Stefani Seibold
2009-12-28 7:42 ` Stefani Seibold
2009-12-28 14:57 ` Andi Kleen
2009-12-28 16:08 ` Stefani Seibold
2009-12-28 17:26 ` Andi Kleen
2009-12-28 20:04 ` Stefani Seibold
2009-12-28 20:40 ` Andi Kleen
2009-12-29 8:40 ` Stefani Seibold
2009-12-29 22:27 ` Dmitry Torokhov
2009-12-30 1:18 ` Vikram Dhillon
2009-12-30 2:08 ` Dmitry Torokhov
2009-12-30 9:29 ` Stefani Seibold
2009-12-30 10:43 ` Dmitry Torokhov
2009-12-30 10:52 ` Stefani Seibold
2009-12-30 11:07 ` Dmitry Torokhov
2009-12-30 11:32 ` Stefani Seibold
2009-12-30 17:29 ` Andy Walls
2009-12-31 7:35 ` Dmitry Torokhov
2009-12-31 8:59 ` Stefani Seibold
2009-12-31 9:33 ` Dmitry Torokhov
2009-12-31 18:03 ` Andy Walls
2009-12-30 17:15 ` Andy Walls
2009-12-28 0:12 ` Roland Dreier
2009-12-28 1:41 ` Andi Kleen
2009-12-28 7:06 ` Stefani Seibold
2009-12-28 14:56 ` Andi Kleen
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=1261950405.25298.30.camel@wall-e \
--to=stefani@seibold.net \
--cc=akpm@osdl.org \
--cc=andi@firstfloor.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox