linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Ulrich Windl <Ulrich.Windl@rz.uni-regensburg.de>
Cc: linux-kernel@vger.kernel.org,
	Mike Christie <michaelc@cs.wisc.edu>,
	iscsi list <open-iscsi@googlegroups.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"James E.J. Bottomley" <JBottomley@parallels.com>,
	Stefani Seibold <stefani@seibold.net>
Subject: Re: Antw: [PATCH 0/5] kfifo cleanup and log based kfifo API
Date: Tue, 8 Jan 2013 23:29:18 +0800	[thread overview]
Message-ID: <20130108152918.GA304@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <50EC4345020000A10000E5D1@gwsmtp1.uni-regensburg.de>

On Tue, Jan 08, 2013 at 04:03:17PM +0100, Ulrich Windl wrote:
> >>> Yuanhan Liu <yuanhan.liu@linux.intel.com> schrieb am 08.01.2013 um 15:57 in
> Nachricht <1357657073-27352-1-git-send-email-yuanhan.liu@linux.intel.com>:
> 
> [...]
> > My proposal is to replace kfifo_init with kfifo_alloc, where it
> > allocate buffer and maintain fifo size inside kfifo. Then we can
> > remove buggy kfifo_init.
> [...]
> 
> Spontaneously I feel that emitting a critical message if the requested size is not a power of two would be a good idea,

Hi Ulrich,

If we can emit all such critical message at compile time, it would
be good then. But we can't, as fifo size is runtime determinated in
quite many cases.

> as well as (in that case) rounding up to the next power of two instead of rounding down seems not too stupid ;-)

There are 2 issues, first, you just can't round the fifo size up to
power of 2 for kfifo_init as fifo buffer is allocated outside. And if
you do, you may access memory outside the buffer.

Second, round up to power of 2 inside kfifo_alloc and DECLARE_KFIFO is
simple and should work. But it wastes memory(by allocating more memory)
without notifing caller.

So, per discussed with Stefani, we better to change the API be log aware
to root fix this kind of potential issue.


Thanks.

	--yliu

      reply	other threads:[~2013-01-08 15:28 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-08 14:57 [PATCH 0/5] kfifo cleanup and log based kfifo API Yuanhan Liu
2013-01-08 14:57 ` [PATCH 1/5] kfifo: remove unnecessary type check Yuanhan Liu
2013-01-08 21:51   ` Stefani Seibold
2013-01-09  2:35     ` Yuanhan Liu
2013-01-09 15:29       ` Stefani Seibold
2013-01-10  7:12         ` Yuanhan Liu
2013-01-08 14:57 ` [PATCH 2/5] libsrp: replace kfifo_init with kfifo_alloc Yuanhan Liu
2013-01-08 14:57 ` [PATCH 3/5] libiscsi: " Yuanhan Liu
2013-01-08 14:57 ` [PATCH 4/5] kfifo: remove kfifo_init Yuanhan Liu
2013-01-08 14:57 ` [PATCH 5/5] kfifo: log based kfifo API Yuanhan Liu
2013-01-08 18:16   ` Dmitry Torokhov
2013-01-08 21:10     ` Andy Walls
2013-01-09  2:42     ` Yuanhan Liu
2013-01-08 15:03 ` Antw: [PATCH 0/5] kfifo cleanup and " Ulrich Windl
2013-01-08 15:29   ` Yuanhan Liu [this message]

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=20130108152918.GA304@yliu-dev.sh.intel.com \
    --to=yuanhan.liu@linux.intel.com \
    --cc=JBottomley@parallels.com \
    --cc=Ulrich.Windl@rz.uni-regensburg.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michaelc@cs.wisc.edu \
    --cc=open-iscsi@googlegroups.com \
    --cc=stefani@seibold.net \
    /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;
as well as URLs for NNTP newsgroup(s).