[PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[Miao Xie]

If the filesystem is mounted to readonly, we should not run scrub.

Signed-off-by: Miao Xie <miaox@cn.fujitsu.com>
---
 fs/btrfs/super.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index ea17f0a..817b3a7 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1151,6 +1151,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
 		/* pause restriper - we want to resume on remount to r/w */
 		btrfs_pause_balance(root->fs_info);
 
+		btrfs_scrub_cancel(root);
+
 		ret = btrfs_commit_super(root);
 		if (ret)
 			goto restore;
-- 
1.7.6.5

Re: [PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[David Sterba]

On Thu, May 17, 2012 at 07:58:21PM +0800, Miao Xie wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1151,6 +1151,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>  		/* pause restriper - we want to resume on remount to r/w */
>  		btrfs_pause_balance(root->fs_info);
>  
> +		btrfs_scrub_cancel(root);

Can we possibly switch scrub to readonly instead ? I'm not sure what's
the 'least surprise here', whether to cancel everything on the
filesystem upon ro-remount or just the minimal set of operations (and
leave the rest running if possible).

Looking at the scrub code, if dev->readonly is set, no repairs are done,
so the only concern is to wait for any outstanding IOs and then switch
to RO.


david

Re: [PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[Miao Xie]

On Fri, 18 May 2012 14:52:07 +0200, David Sterba wrote:
> On Thu, May 17, 2012 at 07:58:21PM +0800, Miao Xie wrote:
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -1151,6 +1151,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>>  		/* pause restriper - we want to resume on remount to r/w */
>>  		btrfs_pause_balance(root->fs_info);
>>  
>> +		btrfs_scrub_cancel(root);
> 
> Can we possibly switch scrub to readonly instead ? I'm not sure what's
> the 'least surprise here', whether to cancel everything on the
> filesystem upon ro-remount or just the minimal set of operations (and
> leave the rest running if possible).

I don't think it is better to switch scrub to readonly soundlessly, because
it is not the operation that the users expect if they don't choose the readonly
mode.

BTW, I think we needn't cancel the readonly scrub on the filesystem upon ro-remount.

Thanks
Miao

> 
> Looking at the scrub code, if dev->readonly is set, no repairs are done,
> so the only concern is to wait for any outstanding IOs and then switch
> to RO.
> 
> 
> david
> 

Re: [PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[Stefan Behrens]

On Mon, 21 May 2012 13:34:05 +0800, Miao Xie wrote:
> On Fri, 18 May 2012 14:52:07 +0200, David Sterba wrote:
>> On Thu, May 17, 2012 at 07:58:21PM +0800, Miao Xie wrote:
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -1151,6 +1151,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>>>  		/* pause restriper - we want to resume on remount to r/w */
>>>  		btrfs_pause_balance(root->fs_info);
>>>  
>>> +		btrfs_scrub_cancel(root);
>>
>> Can we possibly switch scrub to readonly instead ? I'm not sure what's
>> the 'least surprise here', whether to cancel everything on the
>> filesystem upon ro-remount or just the minimal set of operations (and
>> leave the rest running if possible).
> 
> I don't think it is better to switch scrub to readonly soundlessly, because
> it is not the operation that the users expect if they don't choose the readonly
> mode.

Since David put me on CC for this thread, I state my humble opinion:
If I manually remount read-only, I would expect that scrub is switched
into read-only mode as well, not canceled. Plus a log message about this
change.


> BTW, I think we needn't cancel the readonly scrub on the filesystem upon ro-remount.
> 
> Thanks
> Miao
> 
>>
>> Looking at the scrub code, if dev->readonly is set, no repairs are done,
>> so the only concern is to wait for any outstanding IOs and then switch
>> to RO.
>>
>>
>> david

Re: [PATCH 4/5] Btrfs: cancel the scrub when remounting a fs to ro

[Arne Jansen]

On 21.05.2012 07:34, Miao Xie wrote:
> On Fri, 18 May 2012 14:52:07 +0200, David Sterba wrote:
>> On Thu, May 17, 2012 at 07:58:21PM +0800, Miao Xie wrote:
>>> --- a/fs/btrfs/super.c
>>> +++ b/fs/btrfs/super.c
>>> @@ -1151,6 +1151,8 @@ static int btrfs_remount(struct super_block *sb, int *flags, char *data)
>>>  		/* pause restriper - we want to resume on remount to r/w */
>>>  		btrfs_pause_balance(root->fs_info);
>>>  
>>> +		btrfs_scrub_cancel(root);
>>
>> Can we possibly switch scrub to readonly instead ? I'm not sure what's
>> the 'least surprise here', whether to cancel everything on the
>> filesystem upon ro-remount or just the minimal set of operations (and
>> leave the rest running if possible).
> 
> I don't think it is better to switch scrub to readonly soundlessly, because
> it is not the operation that the users expect if they don't choose the readonly
> mode.

I'm not even sure the user expects any changes here. I'd opt for letting
the scrub continue r/w.

-Arne

> 
> BTW, I think we needn't cancel the readonly scrub on the filesystem upon ro-remount.
> 
> Thanks
> Miao
> 
>>
>> Looking at the scrub code, if dev->readonly is set, no repairs are done,
>> so the only concern is to wait for any outstanding IOs and then switch
>> to RO.
>>
>>
>> david
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html