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