On Tue, Feb 06, 2007 at 12:21:40AM -0800, Andrew Morton wrote:
On Tue, 6 Feb 2007 09:02:23 +0100 (CET) Nick Piggin [EMAIL PROTECTED]
wrote:
__block_write_full_page is calling SetPageUptodate without the page locked.
This is unusual, but not incorrect, as PG_writeback is still set.
However with the previous patch, this is now a problem: so don't bother
setting the page uptodate in this case (it is weird that the write path
does such a thing anyway). Instead just leave it to the read side to bring
the page uptodate when it notices that all buffers are uptodate.
Signed-off-by: Nick Piggin [EMAIL PROTECTED]
Index: linux-2.6/fs/buffer.c
===
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -1679,6 +1679,7 @@ static int __block_write_full_page(struc
*/
BUG_ON(PageWriteback(page));
set_page_writeback(page);
+ unlock_page(page);
do {
struct buffer_head *next = bh-b_this_page;
@@ -1688,7 +1689,6 @@ static int __block_write_full_page(struc
}
bh = next;
} while (bh != head);
- unlock_page(page);
err = 0;
done:
Why this change? Without looking at it too hard, it seems that if
submit_bh() completes synchronously, this thread can end up playing with
the buffers on a non-locked, non-PageWriteback page. Someone else could
whip the buffers away and oops?
Hmm, it definitely shouldn't be there, it leaked in from another patch
to bring partiy with the error handling...
Here is an updated patch.
--
__block_write_full_page is calling SetPageUptodate without the page locked.
This is unusual, but not incorrect, as PG_writeback is still set.
However with the previous patch, this is now a problem: so don't bother
setting the page uptodate in this case (it is weird that the write path
does such a thing anyway). Instead just leave it to the read side to bring
the page uptodate when it notices that all buffers are uptodate.
Signed-off-by: Nick Piggin [EMAIL PROTECTED]
Index: linux-2.6/fs/buffer.c
===
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -1698,17 +1698,8 @@ done:
* clean. Someone wrote them back by hand with
* ll_rw_block/submit_bh. A rare case.
*/
- int uptodate = 1;
- do {
- if (!buffer_uptodate(bh)) {
- uptodate = 0;
- break;
- }
- bh = bh-b_this_page;
- } while (bh != head);
- if (uptodate)
- SetPageUptodate(page);
end_page_writeback(page);
+
/*
* The page and buffer_heads can be released at any time from
* here on.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
