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