From b781c01f0279b3687292d2a8990769396014e12e Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Sun, 30 May 2010 23:23:26 +0000 Subject: [PATCH] writeback: Fix fall-out of initial fix for the infamous slow-umount bug Patch fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch fixed but introduced some regressions. Apply fix-ups: - "writeback: Update dirty flags in two step" fixes behaviour in some corner-case - "writeback: ensure that WB_SYNC_NONE writeback with sb pinned is sync" fixes an oops - "writeback: fix non-integrity write-back" fixes a trivial bug in the previous change Also update fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch with the final upstream commit information. svn path=/dists/trunk/linux-2.6/; revision=15809 --- debian/changelog | 4 + ...-pass-in-whether-sb-is-pinned-or-not.patch | 28 ++++-- ...back-Update-dirty-flags-in-two-steps.patch | 57 ++++++++++++ ...ONE-writeback-with-sb-pinned-is-sync.patch | 93 +++++++++++++++++++ ...iteback-fix-non-integrity-write-back.patch | 28 ++++++ debian/patches/series/1~experimental.2 | 3 + 6 files changed, 206 insertions(+), 7 deletions(-) create mode 100644 debian/patches/bugfix/all/writeback-Update-dirty-flags-in-two-steps.patch create mode 100644 debian/patches/bugfix/all/writeback-ensure-WB_SYNC_NONE-writeback-with-sb-pinned-is-sync.patch create mode 100644 debian/patches/bugfix/all/writeback-fix-non-integrity-write-back.patch diff --git a/debian/changelog b/debian/changelog index 233dda6ee..9f0cd686c 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,10 @@ linux-2.6 (2.6.34-1~experimental.2) UNRELEASED; urgency=low [ Ben Hutchings ] * [x86] Reenable rtl8192su, accidentally disabled in previous version (Closes: #580740) + * writeback: Update dirty flags in two steps + * writeback: ensure that WB_SYNC_NONE writeback with sb pinned is sync + (Closes: #582808) + * writeback: fix non-integrity write-back [ maximilian attems ] * topconfig enable CFQ_GROUP_IOSCHED, MFD_WM8994, REGULATOR_MAX8649, diff --git a/debian/patches/bugfix/all/fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch b/debian/patches/bugfix/all/fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch index dc7178674..4c0211d7e 100644 --- a/debian/patches/bugfix/all/fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch +++ b/debian/patches/bugfix/all/fs-explicitly-pass-in-whether-sb-is-pinned-or-not.patch @@ -1,13 +1,27 @@ -From: Jens Axboe -Date: 2010-05-06 07:08:55 -Subject: Explicitly pass in whether sb is pinned or not +From: Jens Axboe +Date: Mon, 17 May 2010 12:55:07 +0200 +Subject: [PATCH] writeback: fix WB_SYNC_NONE writeback from umount +commit e913fc825dc685a444cb4c1d0f9d32f372f59861 upstream. -Revision two of this patch. I got rid of WB_SYNC_NONE_PIN, since we should not -further muddy the sync type and whether or not the sb is pinned. Instead lets -add a specific flag for this. +When umount calls sync_filesystem(), we first do a WB_SYNC_NONE +writeback to kick off writeback of pending dirty inodes, then follow +that up with a WB_SYNC_ALL to wait for it. Since umount already holds +the sb s_umount mutex, WB_SYNC_NONE ends up doing nothing and all +writeback happens as WB_SYNC_ALL. This can greatly slow down umount, +since WB_SYNC_ALL writeback is a data integrity operation and thus +a bigger hammer than simple WB_SYNC_NONE. For barrier aware file systems +it's a lot slower. -BZ: 15906, Launchpad: 543617 +Signed-off-by: Jens Axboe +[maks: Use earlier version for 2.6.34] +--- + fs/fs-writeback.c | 48 +++++++++++++++++++++++++++++++++--------- + fs/sync.c | 2 +- + include/linux/backing-dev.h | 2 +- + include/linux/writeback.h | 10 +++++++++ + mm/page-writeback.c | 4 +- + 5 files changed, 51 insertions(+), 15 deletions(-) diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c index 4b37f7c..c9ac9cb 100644 diff --git a/debian/patches/bugfix/all/writeback-Update-dirty-flags-in-two-steps.patch b/debian/patches/bugfix/all/writeback-Update-dirty-flags-in-two-steps.patch new file mode 100644 index 000000000..2761b3774 --- /dev/null +++ b/debian/patches/bugfix/all/writeback-Update-dirty-flags-in-two-steps.patch @@ -0,0 +1,57 @@ +From: Dmitry Monakhov +Date: Fri, 7 May 2010 13:35:44 +0400 +Subject: [PATCH 3/5] writeback: Update dirty flags in two steps + +commit 5547e8aac6f71505d621a612de2fca0dd988b439 upstream. + +Filesystems with delalloc support may dirty inode during writepages. +As result inode will have dirty metadata flags even after write_inode. +In fact we have two dedicated functions for proper data and metadata +writeback. It is reasonable to separate flags updates in two stages. + +https://bugzilla.kernel.org/show_bug.cgi?id=15906 + +Signed-off-by: Dmitry Monakhov +Reviewed-by: Christoph Hellwig +Signed-off-by: Jens Axboe +--- + fs/fs-writeback.c | 15 +++++++++++---- + 1 files changed, 11 insertions(+), 4 deletions(-) + +diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c +index 67db897..0f62957 100644 +--- a/fs/fs-writeback.c ++++ b/fs/fs-writeback.c +@@ -460,11 +460,9 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc) + + BUG_ON(inode->i_state & I_SYNC); + +- /* Set I_SYNC, reset I_DIRTY */ +- dirty = inode->i_state & I_DIRTY; ++ /* Set I_SYNC, reset I_DIRTY_PAGES */ + inode->i_state |= I_SYNC; +- inode->i_state &= ~I_DIRTY; +- ++ inode->i_state &= ~I_DIRTY_PAGES; + spin_unlock(&inode_lock); + + ret = do_writepages(mapping, wbc); +@@ -480,6 +478,15 @@ writeback_single_inode(struct inode *inode, struct writeback_control *wbc) + ret = err; + } + ++ /* ++ * Some filesystems may redirty the inode during the writeback ++ * due to delalloc, clear dirty metadata flags right before ++ * write_inode() ++ */ ++ spin_lock(&inode_lock); ++ dirty = inode->i_state & I_DIRTY; ++ inode->i_state &= ~(I_DIRTY_SYNC | I_DIRTY_DATASYNC); ++ spin_unlock(&inode_lock); + /* Don't write the inode if only I_DIRTY_PAGES was set */ + if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) { + int err = write_inode(inode, wbc); +-- +1.7.1 + diff --git a/debian/patches/bugfix/all/writeback-ensure-WB_SYNC_NONE-writeback-with-sb-pinned-is-sync.patch b/debian/patches/bugfix/all/writeback-ensure-WB_SYNC_NONE-writeback-with-sb-pinned-is-sync.patch new file mode 100644 index 000000000..e8ac98f6b --- /dev/null +++ b/debian/patches/bugfix/all/writeback-ensure-WB_SYNC_NONE-writeback-with-sb-pinned-is-sync.patch @@ -0,0 +1,93 @@ +From: Jens Axboe +Date: Tue, 18 May 2010 14:29:29 +0200 +Subject: [PATCH 4/5] writeback: ensure that WB_SYNC_NONE writeback with sb pinned is sync + +commit 7c8a3554c683f512dbcee26faedb42e4c05f12fa upstream. + +Even if the writeout itself isn't a data integrity operation, we need +to ensure that the caller doesn't drop the sb umount sem before we +have actually done the writeback. + +This is a fixup for commit e913fc82. + +Signed-off-by: Jens Axboe +--- + fs/fs-writeback.c | 16 +++++++++++----- + 1 files changed, 11 insertions(+), 5 deletions(-) + +diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c +index 0f62957..76f546d 100644 +--- a/fs/fs-writeback.c ++++ b/fs/fs-writeback.c +@@ -193,7 +193,8 @@ static void bdi_wait_on_work_clear(struct bdi_work *work) + } + + static void bdi_alloc_queue_work(struct backing_dev_info *bdi, +- struct wb_writeback_args *args) ++ struct wb_writeback_args *args, ++ int wait) + { + struct bdi_work *work; + +@@ -205,6 +206,8 @@ static void bdi_alloc_queue_work(struct backing_dev_info *bdi, + if (work) { + bdi_work_init(work, args); + bdi_queue_work(bdi, work); ++ if (wait) ++ bdi_wait_on_work_clear(work); + } else { + struct bdi_writeback *wb = &bdi->wb; + +@@ -279,7 +282,7 @@ void bdi_start_writeback(struct backing_dev_info *bdi, struct super_block *sb, + args.for_background = 1; + } + +- bdi_alloc_queue_work(bdi, &args); ++ bdi_alloc_queue_work(bdi, &args, sb_locked); + } + + /* +@@ -909,6 +912,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait) + + while ((work = get_next_work_item(bdi, wb)) != NULL) { + struct wb_writeback_args args = work->args; ++ int post_clear; + + /* + * Override sync mode, in case we must wait for completion +@@ -916,11 +920,13 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait) + if (force_wait) + work->args.sync_mode = args.sync_mode = WB_SYNC_ALL; + ++ post_clear = WB_SYNC_ALL || args.sb_pinned; ++ + /* + * If this isn't a data integrity operation, just notify + * that we have seen this work and we are now starting it. + */ +- if (args.sync_mode == WB_SYNC_NONE) ++ if (!post_clear) + wb_clear_pending(wb, work); + + wrote += wb_writeback(wb, &args); +@@ -929,7 +935,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait) + * This is a data integrity writeback, so only do the + * notification when we have completed the work. + */ +- if (args.sync_mode == WB_SYNC_ALL) ++ if (post_clear) + wb_clear_pending(wb, work); + } + +@@ -1000,7 +1006,7 @@ static void bdi_writeback_all(struct super_block *sb, long nr_pages) + if (!bdi_has_dirty_io(bdi)) + continue; + +- bdi_alloc_queue_work(bdi, &args); ++ bdi_alloc_queue_work(bdi, &args, 0); + } + + rcu_read_unlock(); +-- +1.7.1 + diff --git a/debian/patches/bugfix/all/writeback-fix-non-integrity-write-back.patch b/debian/patches/bugfix/all/writeback-fix-non-integrity-write-back.patch new file mode 100644 index 000000000..d62579ae0 --- /dev/null +++ b/debian/patches/bugfix/all/writeback-fix-non-integrity-write-back.patch @@ -0,0 +1,28 @@ +From: Artem Bityutskiy +Subject: [PATCH] writeback: fix non-integrity write-back +Date: Wed, 26 May 2010 16:08:40 +0300 + +This is a fix for commit 7c8a3554. Note, I only compile-tested +this. + +Signed-off-by: Artem Bityutskiy +--- + fs/fs-writeback.c | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c +index ea8592b..0242855 100644 +--- a/fs/fs-writeback.c ++++ b/fs/fs-writeback.c +@@ -920,7 +920,7 @@ long wb_do_writeback(struct bdi_writeback *wb, int force_wait) + if (force_wait) + work->args.sync_mode = args.sync_mode = WB_SYNC_ALL; + +- post_clear = WB_SYNC_ALL || args.sb_pinned; ++ post_clear = args.sync_mode == WB_SYNC_ALL || args.sb_pinned; + + /* + * If this isn't a data integrity operation, just notify +-- +1.6.6.1 + diff --git a/debian/patches/series/1~experimental.2 b/debian/patches/series/1~experimental.2 index 507b981e0..27a7a58d6 100644 --- a/debian/patches/series/1~experimental.2 +++ b/debian/patches/series/1~experimental.2 @@ -1 +1,4 @@ - debian/dfsg/drivers-staging-rtl8192su-disable.patch ++ bugfix/all/writeback-Update-dirty-flags-in-two-steps.patch ++ bugfix/all/writeback-ensure-WB_SYNC_NONE-writeback-with-sb-pinned-is-sync.patch ++ bugfix/all/writeback-fix-non-integrity-write-back.patch