On 03/24/2014 03:07 PM, Benjamin LaHaise wrote:

On Mon, Mar 24, 2014 at 02:22:06PM -0400, Sasha Levin wrote:

quoted

On 03/21/2014 02:35 PM, Benjamin LaHaise wrote:

quoted

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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>