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

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
--
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

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
--
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

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
> 
--
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

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
--
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

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
--
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