Skip to contentSkip to conversation

xfs: handle negative wbc->nr_to_write during sync writeback

PATCH In review Dave Chinner 2 participants 2 messages Aug 23, 2010 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs: handle negative wbc->nr_to_write during sync writeback

From: Dave Chinner <[email protected]>

During data integrity (WB_SYNC_ALL) writeback, wbc->nr_to_write will go negative on inodes with more than 1024 dirty pages due to implementation details of write_cache_pages(). Currently XFS will abort page clustering in writeback once nr_to_write drops below zero, and so for data integrity writeback we will do very inefficient page at a time allocation and IO submission for inodes with large numbers of dirty pages.

Fix this by only aborting the page clustering code when wbc->nr_to_write is negative and the sync mode is WB_SYNC_NONE.

---
 fs/xfs/linux-2.6/xfs_aops.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fs/xfs/linux-2.6/xfs_aops.c +2 -2
index 15412fe..528be1b 100644
@@ -852,8 +852,8 @@ xfs_convert_page(
 		SetPageUptodate(page);
 
 	if (count) {
-		wbc->nr_to_write--;
-		if (wbc->nr_to_write <= 0)
+		if (--wbc->nr_to_write <= 0 &&
+		    wbc->sync_mode == WB_SYNC_NONE)
 			done = 1;
 	}
 	xfs_start_page_writeback(page, !page_dirty, count);
Signature · 6 lines
1.7.1

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Dave Chinner

From: Dave Chinner <[email protected]>

During data integrity (WB_SYNC_ALL) writeback, wbc->nr_to_write will go negative on inodes with more than 1024 dirty pages due to implementation details of write_cache_pages(). Currently XFS will abort page clustering in writeback once nr_to_write drops below zero, and so for data integrity writeback we will do very inefficient page at a time allocation and IO submission for inodes with large numbers of dirty pages.

Quoted 2 lines

Fix this by only aborting the page clustering code when wbc->nr_to_write is negative and the sync mode is WB_SYNC_NONE.

Looks okay, but I wonder why if we should remove the check entirely. We're clustering inside the same extent, and we already have a cap for the extent size. That means the additional I/O basically is for free and we really shouldn't let the writeback code restrict us here.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs