On Thu, Mar 27, 2014 at 11:16 AM, Linus Torvalds wrote: > > It would all be cleaner if all the setup was done with the > ctx->ring_lock held (you can even *initialize* it to the locked state, > since this is the function that allocates it!) and then it would just > be unlocked when done. Attached is a TOTALLY UNTESTED patch based on Ben's one that does at least this minimal cleanup, in addition to dropping the completion_lock in aio_free_ring() in favor of instead just doing the "put_aio_ring_file()" first. I do want to stress that "untested" part. I'm not going to commit this, because I don't have any real test-cases even if it were to boot and work for me otherwise. I can't say that I like the locking. It really seems totally mis-designed to me. For example, the first completion_lock in aio_migratepage() seems to be total BS - it's locking against other migrations, but that's what "mapping->private_lock" (and now "ctx->ring_lock") protect against. The *second* completion_lock use in aio_migratepage() is actually valid: we can't copy the page contents to a new one when a completion might change the ring tail data, because then the change might be done to the old page but not the new page. But there the "check if things haven't changed" is bogus, since we've held the ring_lock. I did *not* clean up that part. But it's an example of how the locking here seems to be more "voodoo programming" than actually thought about and designed. Please, somebody who has test-cases look at this, ok? Linus