From mboxrd@z Thu Jan 1 00:00:00 1970 From: Benjamin LaHaise Subject: Re: [PATCH RFC] fs/aio: fix sleeping while TASK_INTERRUPTIBLE Date: Tue, 23 Dec 2014 16:58:47 -0500 Message-ID: <20141223215847.GD17185@kvack.org> References: <20141223001619.GA26385@ret.masoncoding.com> <20141223184328.GB17185@kvack.org> <1419360926.13012.12@mail.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, linux-aio@kvack.org, Peter Zijlstra To: Chris Mason Return-path: Content-Disposition: inline In-Reply-To: <1419360926.13012.12@mail.thefacebook.com> Sender: owner-linux-aio@kvack.org List-Id: linux-fsdevel.vger.kernel.org Hi Chris, On Tue, Dec 23, 2014 at 01:55:26PM -0500, Chris Mason wrote: > Works for me, the patch is mostly a (somewhat commented) list of all > the places we're currently doing it wrong. I think the following change should suffice to fix this issue, and it's a lot easier to review, too. I've given this a quick test, and it works for me. I do have one concern: is it safe to call mutex_lock() when the current task is already on other wait queues? If the answer is no, then it may be necessary to convert ->ring_lock back into spinlock as it was prior to 3.10 to avoid using mutex_lock(). The same question applies to kmap() and copy_to_user(), and those concerns might have implications across the rest of the kernel. Thoughts? -ben -- "Thought is the essence of where you are now." diff --git a/fs/aio.c b/fs/aio.c index 1b7893e..a25bd96 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -1140,7 +1140,14 @@ static long aio_read_events_ring(struct kioctx *ctx, long ret = 0; int copy_ret; - mutex_lock(&ctx->ring_lock); + /* Avoid clobbering current->state during mutex_lock(). */ + if (!mutex_trylock(&ctx->ring_lock)) { + long state = current->state; + + __set_current_state(TASK_RUNNING); + mutex_lock(&ctx->ring_lock); + __set_current_state(state); + } /* Access to ->ring_pages here is protected by ctx->ring_lock. */ ring = kmap_atomic(ctx->ring_pages[0]); @@ -1179,6 +1186,7 @@ static long aio_read_events_ring(struct kioctx *ctx, page = ctx->ring_pages[pos / AIO_EVENTS_PER_PAGE]; pos %= AIO_EVENTS_PER_PAGE; + __set_current_state(TASK_RUNNING); ev = kmap(page); copy_ret = copy_to_user(event + ret, ev + pos, sizeof(*ev) * avail); -- To unsubscribe, send a message with 'unsubscribe linux-aio' in the body to majordomo@kvack.org. For more info on Linux AIO, see: http://www.kvack.org/aio/ Don't email: aart@kvack.org