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 S965336AbbBCXeZ (ORCPT <rfc822;w@1wt.eu>);
	Tue, 3 Feb 2015 18:34:25 -0500
Received: from kanga.kvack.org ([205.233.56.17]:40685 "EHLO kanga.kvack.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S965223AbbBCXeV (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Tue, 3 Feb 2015 18:34:21 -0500
Date: Tue, 3 Feb 2015 18:34:18 -0500
From: Benjamin LaHaise <bcrl@kvack.org>
To: Dave Chinner <david@fromorbit.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>, linux-aio@kvack.org,
        Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] aio: fix sleeping while TASK_INTERRUPTIBLE
Message-ID: <20150203233418.GC14400@kvack.org>
References: <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> <20150202052944.GF4251@dastard> <CA+55aFwvEcq-rAbqF2qTut=kJgFZZnhHptoPi6FSVrF4+1tBNA@mail.gmail.com> <20150202055429.GH4251@dastard> <CA+55aFy2sroiyKN-Z5YgLv0ffqezFd=UP4752_YxMiW2uTGaSQ@mail.gmail.com> <20150203222312.GO6282@dastard>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150203222312.GO6282@dastard>
User-Agent: Mutt/1.4.2.2i
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Feb 04, 2015 at 09:23:12AM +1100, Dave Chinner wrote:
> On Mon, Feb 02, 2015 at 10:45:42AM -0800, Linus Torvalds wrote:
> > On Sun, Feb 1, 2015 at 9:54 PM, Dave Chinner <david@fromorbit.com> wrote:
> > >
> > > Simple enough - the patch below removes the warning from generic/036
> > > for me.
> > 
> > So because this is a debugging thing, I'd actually prefer these
> > "sched_annotate_sleep()" calls to always come with short  comments in
> > code why they exist and why they are fine.
> 
> Ok, I just copied the existing users which don't have any comments.
> 
> > In this case, it might be as simple as
> > 
> >  "If the mutex blocks and wakes us up, the loop in
> > wait_event_interruptible_hrtimeout() will just schedule without
> > sleeping and repeat. The ting-lock doesn't block often enough for this
> > to be a performance issue".
> > 
> > or perhaps just point to the comment in read_events().
> 
> Both. New patch below.

I've added my Signed-off-by and applied it to my aio-fixes tree.  I'll send 
a pull for this later this evening once I commit a fix for an mremap case 
that just got pointed out earlier today as well.

		-ben

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 
> aio: annotate aio_read_event_ring for sleep patterns
> 
> From: Dave Chinner <dchinner@redhat.com>
> 
> Under CONFIG_DEBUG_ATOMIC_SLEEP=y, aio_read_event_ring() will throw
> warnings like the following due to being called from wait_event
> context:
> 
>  WARNING: CPU: 0 PID: 16006 at kernel/sched/core.c:7300 __might_sleep+0x7f/0x90()
>  do not call blocking ops when !TASK_RUNNING; state=1 set at [<ffffffff810d85a3>] prepare_to_wait_event+0x63/0x110
>  Modules linked in:
>  CPU: 0 PID: 16006 Comm: aio-dio-fcntl-r Not tainted 3.19.0-rc6-dgc+ #705
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>   ffffffff821c0372 ffff88003c117cd8 ffffffff81daf2bd 000000000000d8d8
>   ffff88003c117d28 ffff88003c117d18 ffffffff8109beda ffff88003c117cf8
>   ffffffff821c115e 0000000000000061 0000000000000000 00007ffffe4aa300
>  Call Trace:
>   [<ffffffff81daf2bd>] dump_stack+0x4c/0x65
>   [<ffffffff8109beda>] warn_slowpath_common+0x8a/0xc0
>   [<ffffffff8109bf56>] warn_slowpath_fmt+0x46/0x50
>   [<ffffffff810d85a3>] ? prepare_to_wait_event+0x63/0x110
>   [<ffffffff810d85a3>] ? prepare_to_wait_event+0x63/0x110
>   [<ffffffff810bdfcf>] __might_sleep+0x7f/0x90
>   [<ffffffff81db8344>] mutex_lock+0x24/0x45
>   [<ffffffff81216b7c>] aio_read_events+0x4c/0x290
>   [<ffffffff81216fac>] read_events+0x1ec/0x220
>   [<ffffffff810d8650>] ? prepare_to_wait_event+0x110/0x110
>   [<ffffffff810fdb10>] ? hrtimer_get_res+0x50/0x50
>   [<ffffffff8121899d>] SyS_io_getevents+0x4d/0xb0
>   [<ffffffff81dba5a9>] system_call_fastpath+0x12/0x17
>  ---[ end trace bde69eaf655a4fea ]---
> 
> There is not actually a bug here, so annotate the code to tell the
> debug logic that everything is just fine and not to fire a false
> positive.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
> V2: added comment to explain the annotation.
> 
>  fs/aio.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index 1b7893e..327ef6d 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1140,6 +1140,13 @@ static long aio_read_events_ring(struct kioctx *ctx,
>  	long ret = 0;
>  	int copy_ret;
>  
> +	/*
> +	 * The mutex can block and wake us up and that will cause
> +	 * wait_event_interruptible_hrtimeout() to schedule without sleeping
> +	 * and repeat. This should be rare enough that it doesn't cause
> +	 * peformance issues. See the comment in read_events() for more detail.
> +	 */
> +	sched_annotate_sleep();
>  	mutex_lock(&ctx->ring_lock);
>  
>  	/* Access to ->ring_pages here is protected by ctx->ring_lock. */

-- 
"Thought is the essence of where you are now."