Patchwork [2/2] writeback: fix __sync_filesystem(sb, 0) on umount

login
register
mail settings
Submitter Surbhi Palande
Date May 6, 2010, 10:35 a.m.
Message ID <d4432e2c1344006766a9a7ae95ea8aab4375be06.1273139148.git.surbhi.palande@canonical.com>
Download mbox | patch
Permalink /patch/51824/
State Superseded
Delegated to: Stefan Bader
Headers show

Comments

Surbhi Palande - May 6, 2010, 10:35 a.m.
From: Dmitry Monakhov <dmonakhov@openvz.org>

BugLink: http://launchpad.net/bugs/543617

__sync_filesystem(sb, 0) is no longer works on umount because sb can
not be pined, Because s_mount sem is downed for write and s_root is NULL.
In fact umount is a special case similar to WB_SYNC_ALL, where
sb is pinned already.

BADCOMMIT: 03ba3782e8dcc5b0e1efe440d33084f066e38cae

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
(cherry-picked from https://bugzilla.kernel.org/attachment.cgi?id=26224)

Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
---
 fs/fs-writeback.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Stefan Bader - May 6, 2010, 12:51 p.m.
On 05/06/2010 12:35 PM, Surbhi Palande wrote:
> From: Dmitry Monakhov <dmonakhov@openvz.org>
> 
> BugLink: http://launchpad.net/bugs/543617
> 
> __sync_filesystem(sb, 0) is no longer works on umount because sb can
> not be pined, Because s_mount sem is downed for write and s_root is NULL.
> In fact umount is a special case similar to WB_SYNC_ALL, where
> sb is pinned already.
> 
> BADCOMMIT: 03ba3782e8dcc5b0e1efe440d33084f066e38cae
> 
> Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
> (cherry-picked from https://bugzilla.kernel.org/attachment.cgi?id=26224)
> 
> Signed-off-by: Surbhi Palande <surbhi.palande@canonical.com>
> ---
>  fs/fs-writeback.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 4102f20..937a8ae 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -583,9 +583,9 @@ static int pin_sb_for_writeback(struct writeback_control *wbc,
>  		unpin_sb_for_writeback(psb);
>  
>  	/*
> -	 * Caller must already hold the ref for this
> +	 * Caller must already hold the ref for integrity sync, and on umount
>  	 */
> -	if (wbc->sync_mode == WB_SYNC_ALL) {
> +	if (wbc->sync_mode == WB_SYNC_ALL || !sb->s_root) {
>  		WARN_ON(!rwsem_is_locked(&sb->s_umount));
>  		return 0;
>  	}
> @@ -597,6 +597,7 @@ static int pin_sb_for_writeback(struct writeback_control *wbc,
>  			spin_unlock(&sb_lock);
>  			goto pinned;
>  		}
> +		WARN_ON(1);

Hm, will users then see a warning with stack trace everytime they unmount?

>  		/*
>  		 * umounted, drop rwsem again and fall through to failure
>  		 */

Patch

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 4102f20..937a8ae 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -583,9 +583,9 @@  static int pin_sb_for_writeback(struct writeback_control *wbc,
 		unpin_sb_for_writeback(psb);
 
 	/*
-	 * Caller must already hold the ref for this
+	 * Caller must already hold the ref for integrity sync, and on umount
 	 */
-	if (wbc->sync_mode == WB_SYNC_ALL) {
+	if (wbc->sync_mode == WB_SYNC_ALL || !sb->s_root) {
 		WARN_ON(!rwsem_is_locked(&sb->s_umount));
 		return 0;
 	}
@@ -597,6 +597,7 @@  static int pin_sb_for_writeback(struct writeback_control *wbc,
 			spin_unlock(&sb_lock);
 			goto pinned;
 		}
+		WARN_ON(1);
 		/*
 		 * umounted, drop rwsem again and fall through to failure
 		 */