handle concurrent block_write_full_page and set_page_dirty
Subsystems (3)
PATCH 7/10 handle concurrent block_write_full_page and set_page_dirty
set_page_dirty() runs without the page lock. So __block_write_full_page() needs to be able to cope with the page's buffers being dirtied concurrently, on another CPU.
Do this with careful ordering and a test-and-set.
=====================================
mm/page-writeback.c renamed from mm/page-writeback.c~writepage-versus-set_page_dirty +2 -0
@@ -480,6 +480,8 @@ int __set_page_dirty_buffers(struct pagedo {if (buffer_uptodate (bh))set_buffer_dirty (bh); +else +buffer_error (); bh = bh->b_this_page; }while (bh != head); }
fs/buffer.c renamed from fs/buffer.c~writepage-versus-set_page_dirty +14 -11
@@ -1288,6 +1288,16 @@ static int __block_write_full_page(struc (1 << BH_Dirty)|(1 << BH_Uptodate)); } +/* +* Be very careful. We have no exclusion from __set_page_dirty_buffers +* here, and the (potentially unmapped) buffers may become dirty at +* any time. If a buffer becomes dirty here after we've inspected it +* then we just miss that fact, and the page stays dirty. +* +* Buffers outside i_size may be dirtied by __set_page_dirty_buffers; +* handle that here by just cleaning them. +*/ + block = page->index << (PAGE_CACHE_SHIFT - inode->i_blkbits); head =page_buffers (page); bh = head;
@@ -1298,15 +1308,11 @@ static int __block_write_full_page(struc*/ do { if (block > last_block) { - if (buffer_dirty(bh)) - buffer_error(); + clear_buffer_dirty(bh); if (buffer_mapped(bh)) buffer_error(); /* - * NOTE: this buffer can only be marked uptodate - * because we know that block_write_full_page has - * zeroed it out. That seems unnecessary and may go - * away. + * The buffer was zeroed by block_write_full_page() */ set_buffer_uptodate(bh); } else if (!buffer_mapped(bh) && buffer_dirty(bh)) {
@@ -1327,11 +1333,9 @@ static int __block_write_full_page(struc do { get_bh(bh); - if (buffer_dirty(bh)) { + if (buffer_mapped(bh) && buffer_dirty(bh)) { lock_buffer(bh); - if (buffer_dirty(bh)) { - if (!buffer_mapped(bh)) - buffer_error(); + if (test_clear_buffer_dirty(bh)) { if (!buffer_uptodate(bh)) buffer_error(); set_buffer_async_io(bh);
@@ -1353,7 +1357,6 @@ static int __block_write_full_page(struc do { struct buffer_head *next = bh->b_this_page; if (buffer_async(bh)) { - clear_buffer_dirty(bh); submit_bh(WRITE, bh); nr_underway++; }
-
fs/buffer.c static int __block_write_full_page(struc · line 1298
Quoted 2 lines · Andrew Morton
fs/buffer.c renamed from fs/buffer.c~writepage-versus-set_page_dirty +2
-0+* Buffers outside i_size may be dirtied by __set_page_dirty_buffers; +* handle that here by just cleaning them.
"Cleaning", to me at least, means writing out as a prelude to setting clean. I'm think meant 'just setting them clean'.
Signature · 1 line
Daniel