Re: [PATCH] aio: ensure access to ctx->ring_pages is correctly serialised

From: Sasha Levin
Date: Tue Mar 25 2014 - 13:48:10 EST


On 03/24/2014 03:07 PM, Benjamin LaHaise wrote:
On Mon, Mar 24, 2014 at 02:22:06PM -0400, Sasha Levin wrote:
On 03/21/2014 02:35 PM, Benjamin LaHaise wrote:
Hi all,

Based on the issues reported by Tang and Gu, I've come up with the an
alternative fix that avoids adding additional locking in the event read
code path. The fix is to take the ring_lock mutex during page migration,
which is already used to syncronize event readers and thus does not add
any new locking requirements in aio_read_events_ring(). I've dropped
the patches from Tang and Gu as a result. This patch is now in my
git://git.kvack.org/~bcrl/aio-next.git tree and will be sent to Linus
once a few other people chime in with their reviews of this change.
Please review Tang, Gu. Thanks!

Hi Benjamin,

This patch seems to trigger:

[ 433.476216] ======================================================
[ 433.478468] [ INFO: possible circular locking dependency detected ]
...

Yeah, that's a problem -- thanks for the report. The ring_lock mutex can't
be nested inside of mmap_sem, as aio_read_events_ring() can take a page
fault while holding ring_mutex. That makes the following change required.
I'll fold this change into the patch that caused this issue.

Yup, that does the trick.

Could you please add something to document why this is a trylock instead of a lock? If
I were reading the code there's no way I'd understand what's the reason behind it
without knowing of this bug report.


Thanks,
Sasha

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/