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.
Thanks for forcing me to rethink this :)

-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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to