From: "Robert P. J. Day" <rpjday@crashcourse.ca>
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: kfifo has temporarily invalid in pointer?
Date: Mon, 15 Mar 2010 10:58:00 -0400 (EDT) [thread overview]
Message-ID: <alpine.LFD.2.00.1003151044590.8729@localhost> (raw)
(i am not trying to be annoyingly obsessive about the kernel kfifo,
i am merely succeeding.)
what appears to be a bit of an oddity WRT kfifo: since a kfifo is
defined with a fixed buffer size, it obviously enqueues and dequeues
in a circular fashion. so, the code to add some data to a kfifo (from
kernel/kfifo.c):
=====
unsigned int kfifo_in(struct kfifo *fifo, const void *from,
unsigned int len)
{
len = min(kfifo_avail(fifo), len);
__kfifo_in_data(fifo, from, len, 0);
__kfifo_add_in(fifo, len);
return len;
}
=====
fair enough -- that first routine adds the data itself, while the
second one correspondingly bumps up the pointer, which could
conceivably wrap around to follow the data, correct? but from
include/linux.kfifo.h:
=====
static inline void __kfifo_add_in(struct kfifo *fifo,
unsigned int off)
{
smp_wmb();
fifo->in += off;
}
=====
note that there is no attempt to check for wraparound -- the new
value of "fifo->in" could (theoretically) be off the end of the
kfifo's buffer. to make a long story short, when one subsequently
tries to *dequeue* data, one eventually invokes:
=====
static inline void __kfifo_out_data(struct kfifo *fifo,
void *to, unsigned int len, unsigned int off)
{
unsigned int l;
/*
* Ensure that we sample the fifo->in index -before- we
* start removing bytes from the kfifo.
*/
smp_rmb();
off = __kfifo_off(fifo, fifo->out + off); <----- there
... snip...
=====
where __kfifo_off is defined as:
=====
static inline unsigned int __kfifo_off(struct kfifo *fifo, unsigned int off)
{
return off & (fifo->size - 1);
}
=====
which is clearly what takes a given offset and *now* adjusts it if it
represents a wraparound.
but that seems to suggest that, between the time data is enqueued
which represents a circular wraparound and the time that data is
dequeued, the value of fifo->in is temporarily rubbish -- it might
have a value that's off the end of the kfifo buffer, no?
admittedly, the code seems to work in that it always takes the above
into account, but it would seem to make a mess of debugging, since if
you were printing out the contents of a kfifo, you could conceivably
read a value of fifo->in that's larger than the buffer size.
am i reading this correctly? should i care?
rday
--
========================================================================
Robert P. J. Day Waterloo, Ontario, CANADA
Linux Consulting, Training and Kernel Pedantry.
Web page: http://crashcourse.ca
Twitter: http://twitter.com/rpjday
========================================================================
next reply other threads:[~2010-03-15 14:59 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-15 14:58 Robert P. J. Day [this message]
2010-03-15 16:06 ` kfifo has temporarily invalid in pointer? Daniel Baluta
2010-03-15 17:17 ` Robert P. J. Day
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=alpine.LFD.2.00.1003151044590.8729@localhost \
--to=rpjday@crashcourse.ca \
--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