Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965336AbbBCXeZ (ORCPT ); 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 ); Tue, 3 Feb 2015 18:34:21 -0500 Date: Tue, 3 Feb 2015 18:34:18 -0500 From: Benjamin LaHaise To: Dave Chinner Cc: Linus Torvalds , linux-aio@kvack.org, Kernel Mailing List Subject: Re: [GIT PULL] aio: fix sleeping while TASK_INTERRUPTIBLE Message-ID: <20150203233418.GC14400@kvack.org> References: <20150201221458.GN2974@kvack.org> <20150202001628.GO2974@kvack.org> <20150202052944.GF4251@dastard> <20150202055429.GH4251@dastard> <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: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4073 Lines: 104 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 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 > > 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 [] 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: > [] dump_stack+0x4c/0x65 > [] warn_slowpath_common+0x8a/0xc0 > [] warn_slowpath_fmt+0x46/0x50 > [] ? prepare_to_wait_event+0x63/0x110 > [] ? prepare_to_wait_event+0x63/0x110 > [] __might_sleep+0x7f/0x90 > [] mutex_lock+0x24/0x45 > [] aio_read_events+0x4c/0x290 > [] read_events+0x1ec/0x220 > [] ? prepare_to_wait_event+0x110/0x110 > [] ? hrtimer_get_res+0x50/0x50 > [] SyS_io_getevents+0x4d/0xb0 > [] 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 > --- > 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." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/