Re: [PATCH v4 4/6] btrfs: sync scrub with commit & device removal

[Arne Jansen <sensille@gmx.net>]

On 24.03.2011 13:58, Arne Jansen wrote:
> On 23.03.2011 18:28, David Sterba wrote:
>> Hi,
>>
>> you are adding a new smp_mb, can you please explain why it's needed and
>> document it?
>>
>> thanks,
>> dave
>>
>> On Fri, Mar 18, 2011 at 04:55:07PM +0100, Arne Jansen wrote:
>>> This adds several synchronizations:
>>>  - for a transaction commit, the scrub gets paused before the
>>>    tree roots are committed until the super are safely on disk
>>>  - during a log commit, scrubbing of supers is disabled
>>>  - on unmount, the scrub gets cancelled
>>>  - on device removal, the scrub for the particular device gets cancelled
>>>
> 
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -1330,6 +1330,8 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path)
>>>  		goto error_undo;
>>>  
>>>  	device->in_fs_metadata = 0;
>>> +	smp_mb();
>>         ^^^^^^^^
> 
> The idea was to disallow any new scrubs so start beyond
> this point, but it turns out this is not strong enough.
> I have to move the check for in_fs_metadata in btrfs_scrub_dev
> inside the scrub_lock. In this case, the smp_mb is still needed,
> as in_fs_metadata is not protected by any lock. I'll add a
> comment.

Thinking more about locking... the smp_mb is not necessary,
because the following cancel_dev aquires a spin_lock, which
implies a barrier.

> 
> -Arne
> 
>>
>>> +	btrfs_scrub_cancel_dev(root, device);
>>>  
>>>  	/*
>>>  	 * the device list mutex makes sure that we don't change
> 
> 
> --
> 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