On Sun, Feb 01, 2015 at 03:33:25PM -0800, Linus Torvalds wrote:
> On Sun, Feb 1, 2015 at 2:14 PM, Benjamin LaHaise <b...@kvack.org> wrote:
> >
> > It's ugly, but it actually is revealing a bug. Spurious wake ups caused
> > by the task already being added to ctx->wait when calling into mutex_lock()
> > could inadvertently cause things to go wrong. I can envision there being
> > code invoked that possibly expects a 1-1 relationship between sleeps and
> > wake ups, which being on the additional wait queue might break.
>
> So I'm looking at it, and I don't see it.
>
> One side uses wait_event_interruptible_hrtimeout(), which waits for
> the return value (or the timeout), and it doesn't matter how many
> times it gets woken up, regardless of what it's waiting for. If it
> gets extra wakeups, it will just go through the loop again.
>
> The other side is just a plain aio_read_events() ->
> aio_read_events_ring(), and that one just reads as many events as it
> can, unless some error happens.
>
> In other words, it really looks like the warning is spurious, and the
> comments about how the semaphore could cause it to loop around but it
> all works look entirely correct.
>
> So no, I don't see it revealing a bug at all. All I see is a spurious warning.
>
> What's the bug you think could happen?
The bug would be in code that gets run via mutex_lock(), kmap(), or (more
likely) in the random mm or filesystem code that could be invoked via
copy_to_user().
If someone has code that works like the following inside of, say, a filesystem
that's doing i/o somewhere:
static void foo_done(struct foo *foo)
{
/* stuff err somewhere */
wake_up(foo->task);
}
void some_random_code_in_some_fs()
{
struct foo *foo;
/* setup the foo */
foo->task = current;
set_current_state(TASK_UNINTERRUPTIBLE);
/* send the foo on to do some other work */
schedule();
/* foo_done should have been called at this point. */
}
When the task in question can receive spurious wake ups, there is the
possibility that schedule() ends up returning without foo_done() having
been called, which is not the case normally (that is, there should be a
one to one relationship between the wake_up and schedule returning in
this case). While I don't immediately see any code that relies on this,
I'm not convinced that every possible code path that can be invoked
(especially via copy_to_user()) does not rely on these semantics. Maybe
I'm just being paranoid, but this is one of the concerns I raised when
this issue came forth. Nobody has addressed it yet, though.
-ben
> Linus
--
"Thought is the essence of where you are now."
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
