On Mon, 2005-01-24 at 16:58, Andrew Morton wrote: > "Darrick J. Wong" <[EMAIL PROTECTED]> wrote: > > > > Andrew Morton wrote: > > > > > So... Will someone be sending a new patch? > > > > Here's a cheesy patch that simply marks the ioctx as dead before > > destroying it. > > super-cheesy, given that `ctx' is an unsigned long. > > > + spin_lock_irq(&ctx->ctx_lock); > > + ctx->dead = 1; > > + spin_unlock_irq(&ctx->ctx_lock); > > + > > Even with this fixed up, the locking looks very odd. > > Needs more work, please. Or we just run with the original patch which I > assume was tested. It's a rare error path and performance won't matter.
The use of 'dead' looks very strange. It is set to 1 in aio_cancel_all() while holding spin_lock_irq(&ctx->ctx_lock); but it is set to 1 in io_destroy() holding write_lock(&mm->ioctx_list_lock); The io_destroy() comment says "Protects against races with itself via ->dead." I assume the race the comment is talking about is multiple threads calling io_destroy() on the same ctx. io_destroy() in only called from sys_io_destroy() or from sys_io_setup() in the failure case that is trying to be fixed. aio_cancel_all() is only called from exit_aio() when the mm is going away. So this path is using 'dead' for something else since the mm cannot go away twice (and there cannot be an io_destroy() in progress or the mm would not be going away). The overloading of 'dead' is ugly and confusing. The use of spin_lock_irq(&ctx->ctx_lock) in sys_io_setup() does not do any good AFAICT. Daniel - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
