From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S932497AbbBBF3t (ORCPT <rfc822;w@1wt.eu>);
	Mon, 2 Feb 2015 00:29:49 -0500
Received: from ipmail06.adl2.internode.on.net ([150.101.137.129]:50881 "EHLO
	ipmail06.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1751951AbbBBF3s (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 2 Feb 2015 00:29:48 -0500
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: Av0WAHEKz1R5LHOVPGdsb2JhbABbgwaBK4Iyg3mrcQEBAQEBAQaYKQICAQECgRVEAQEBAQEGAQEBATg7hA0BBTocIxAIAxgJJQ8FJQMHGhOILNJ+AQEBAQYBAQEBHhiFbIl0B4MWgRMFhUuSYoo1iCiCIQMcgWQqMYJCAQEB
Date: Mon, 2 Feb 2015 16:29:44 +1100
From: Dave Chinner <david@fromorbit.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Benjamin LaHaise <bcrl@kvack.org>, linux-aio@kvack.org,
        Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] aio: fix sleeping while TASK_INTERRUPTIBLE
Message-ID: <20150202052944.GF4251@dastard>
References: <20150201144058.GM2974@kvack.org>
 <CA+55aFx+HVnythC+BpmDMa0Oeq1MxnXXOvs771DXpJ=PBjy1Nw@mail.gmail.com>
 <20150201221458.GN2974@kvack.org>
 <CA+55aFzAPms_orX7TY6zGCg=G-yXCrr5z7LMme7xcrXCAk6JAQ@mail.gmail.com>
 <20150202001628.GO2974@kvack.org>
 <CA+55aFy2URGTL_XKQhEO_4dTc3KNJDm9Z0O_1Gqs4G3TvW7APg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+55aFy2URGTL_XKQhEO_4dTc3KNJDm9Z0O_1Gqs4G3TvW7APg@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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