Re: [GIT PULL] aio: fix sleeping while TASK_INTERRUPTIBLE

[Dave Chinner <david@fromorbit.com>]

On Sun, Feb 01, 2015 at 05:18:17PM -0800, Linus Torvalds wrote:
> On Sun, Feb 1, 2015 at 4:16 PM, Benjamin LaHaise <bcrl@kvack.org> wrote:
> >>
> >> 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().
> 
> Ahh. That would be a bug, yes, but it wouldn't be one in the aio code.
> 
> If somebody just does a "schedule()" and thinks that their own private
> events are the only thing that can wake it up, and doesn't use one of
> the millions of "wait_event_xyz()" variations to actually wait for the
> real completion, that is just buggy. Always. Always has been.
> 
> So I wouldn't worry too much about it. It has never been correct to do
> that, and it's not one of the standard patterns for sleeping anyway.
> Which is not to say that it might not exist in some dank corner of the
> kernel, of course, but you shouldn't write code trying to make buggy
> code like that happy. If we ever find code like that, let's just fix
> it where the bug exists, not try to write odd code in places where it
> isn't.
> 
> And I'd actually be a bit surprised to see that kind of really broken
> code. You really almost have to work at it. All our normal "sleep
> until X" patterns are much more obvious, and it's just _simpler_ to do
> the right thing with "wait_event()" than to mis-code an explicit "set
> task state and then just schedule without actually checking the thing
> you are waiting for".

So what's the outcome here? I'm running v3.19-rc7 kernel and
xfstests::generic/036 is still tripping this warning from the aio
code:

[   23.920785] run fstests generic/036 at 2015-02-02 16:13:01
[   24.168001] xfs_io (4814) used greatest stack depth: 11640 bytes left
[   24.187061] ------------[ cut here ]------------
[   24.187071] WARNING: CPU: 1 PID: 4820 at kernel/sched/core.c:7300 __might_sleep+0x7f/0x90()
[   24.187076] do not call blocking ops when !TASK_RUNNING; state=1 set at [<ffffffff810d85a3>] prepare_to_wait_event+0x63/0x110
[   24.187078] Modules linked in:
[   24.187080] CPU: 1 PID: 4820 Comm: aio-dio-fcntl-r Not tainted 3.19.0-rc7-dgc+ #706
[   24.187081] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[   24.187084]  ffffffff821c0372 ffff8800b84d3cd8 ffffffff81daf2bd 0000000000008c8c
[   24.187085]  ffff8800b84d3d28 ffff8800b84d3d18 ffffffff8109beda ffff8800b84d3cf8
[   24.187087]  ffffffff821c115e 0000000000000061 0000000000000000 00007fff38e95180
[   24.187087] Call Trace:
[   24.187093]  [<ffffffff81daf2bd>] dump_stack+0x4c/0x65
[   24.187096]  [<ffffffff8109beda>] warn_slowpath_common+0x8a/0xc0
[   24.187099]  [<ffffffff8109bf56>] warn_slowpath_fmt+0x46/0x50
[   24.187101]  [<ffffffff810d85a3>] ? prepare_to_wait_event+0x63/0x110
[   24.187103]  [<ffffffff810d85a3>] ? prepare_to_wait_event+0x63/0x110
[   24.187104]  [<ffffffff810bdfcf>] __might_sleep+0x7f/0x90
[   24.187107]  [<ffffffff81db8344>] mutex_lock+0x24/0x45
[   24.187111]  [<ffffffff81216b7c>] aio_read_events+0x4c/0x290
[   24.187113]  [<ffffffff81216fac>] read_events+0x1ec/0x220
[   24.187115]  [<ffffffff810d8650>] ? prepare_to_wait_event+0x110/0x110
[   24.187119]  [<ffffffff810fdb10>] ? hrtimer_get_res+0x50/0x50
[   24.187121]  [<ffffffff8121899d>] SyS_io_getevents+0x4d/0xb0
[   24.187124]  [<ffffffff81130b16>] ? __audit_syscall_exit+0x236/0x2e0
[   24.187127]  [<ffffffff81dba5a9>] system_call_fastpath+0x12/0x17
[   24.187128] ---[ end trace 9f078e8e19f765dd ]---

This test does a aio write loop with a single outstanding
event. i.e:

	while (1) {
		io_prep_pwrite(&iocb, fd, buf, BUF_SIZE, BUF_SIZE);
		err = io_submit(ctx, 1, iocbs);
		if (err)
			.....
		err = io_getevents(ctx, 1, 1, &ev, NULL);
		.....
	}

The kernel should not be throwing warnings on basic regression
tests, so something needs to be fixed...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com