On Fri, 28 Sep 2001, Andrea Arcangeli wrote:
> The deadlock happens in the middle of write_locked_buffers when we hit
> an highmem buffer, so while allocating with GFP_NOHIGHIO we end doing
> sync_page_buffers on any page that isn't highmem, but that incidentally is one of the
> other next buffers in the array that we previously locked in
> write_some_buffers but that aren't in the I/O queue yet (so we'll wait
> forever since they depends on us to be written).
However, your patch is racy:
> --- 2.4.10aa2/fs/buffer.c.~1~ Wed Sep 26 18:45:29 2001
> +++ 2.4.10aa2/fs/buffer.c Fri Sep 28 00:04:44 2001
> @@ -194,6 +194,7 @@
> struct buffer_head * bh = *array++;
> bh->b_end_io = end_buffer_io_sync;
> submit_bh(WRITE, bh);
> + clear_bit(BH_Pending_IO, &bh->b_state);
No way can we clear the bit here, because the submit_bh() may have caused
the buffer to be unlocked and IO to have completed, and it is no longer
"owned" by us - somebody else might have started IO on it and we'd be
clearing the bit for the wrong user.
I would suggest a totally different approach: make the "can we wait for
existing buffer heads" condition a GFP bit the same way the HIGHIO thing
is a GFP bit, and just not set it for GFP_NOHIGHIO.
Thinking about it, I think GFP_NOIO also implies "we must not wait for
other buffers", because that could deadlock for _other_ things too, like
loop and NBD (which use NOIO to make sure that they don't recurse - but
that should also imply not waiting for themselves). The GFP_xxx approach
should fix those deadlocks too.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to firstname.lastname@example.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sun Sep 30 2001 - 21:00:59 EST