fix for slow unmount on ext4 - bug 543617 on lp

Stefan Bader stefan.bader at canonical.com
Mon Jun 7 12:10:43 UTC 2010


Ok, so upstream seems to have officially reverted the changes for now. Given
that the changes in that area seem to have unexpected side effects and we do not
really know the other patch would not do the same. When reverting the
work-around we got huge sync times. IIRC the side effects of that patch were not
as bad timewise (2 seconds?). So maybe for now, we stay with what we got and
wait for the proper upstream solution?

Stefan

On 06/02/2010 03:09 PM, Stefan Bader wrote:
> This is a followup on a previous post which basically consists of the
> revert of 5e1941884c700b7b97bcad52c2d8212ac56a7ebc
>  "UBUNTU: SAUCE: sync before umount to reduce time taken by ext4 umount"
> and then applying a patch from bugzilla
>  "writeback: fix __sync_filesystem(sb, 0) on umount"
> 
> However the second patch never went upstream, while the following two
> (backported to 2.6.32.y) made it. Sadly, when asking Jens Axboe about
> forwarding those to stable, I got the following response:
> 
> Jens Axboe wrote:
>>> I would send the backports to stable but wanted to check with you
>>> beforehand. Would you be ok with a stable submission of these two?
> 
>> I would have said yes a few days ago, but Christoph claims that the
>> patches make xfs test hang. Unfortunately I'm a bit between jobs and
>> don't have proper testing equipment right now, so I had no other option
>> than revert exactly those two commits...
> 
> Now the question is whether we want to go ahead (because we need to
> do the revert as that causes other problems) with the upstream version
> that might cause xfs problems (assuming xfs is not our main fs) or
> take the patch from bugzilla which might have the same problem or others
> yet unknown.
> 
> -Stefan
> 
> ----
> 
> From: Dmitry Monakhov <dmonakhov at 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 at openvz.org>
> (cherry-picked from https://bugzilla.kernel.org/attachment.cgi?id=26224)
> 
> Signed-off-by: Surbhi Palande <surbhi.palande at 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);
>  		/*
>  		 * umounted, drop rwsem again and fall through to failure
>  		 */
> 
> 





More information about the kernel-team mailing list