On Thu, Apr 11, 2013 at 06:30:16PM +0800, Miao Xie wrote:
> In order to avoid this problem, we introduce a lock named super_lock into
> the btrfs_fs_info structure. If we want to update incompat/compat flags
> of the super block, we must hold it.
> 
> +     /*
> +      * Used to protect the incompat_flags, compat_flags, compat_ro_flags
> +      * when they are updated.

> +     spinlock_t super_lock;

The lock name is too general for protecting just *_flags, do you have
plans to add more items from superblock under this lock?  If no, I
suggest to pick a different name.

> @@ -3663,8 +3674,15 @@ static inline void __btrfs_set_fs_incompat(struct 
> btrfs_fs_info *fs_info,
>       disk_super = fs_info->super_copy;
>       features = btrfs_super_incompat_flags(disk_super);
>       if (!(features & flag)) {
> -             features |= flag;
> -             btrfs_set_super_incompat_flags(disk_super, features);
> +             spin_lock(&fs_info->super_lock);
> +             features = btrfs_super_incompat_flags(disk_super);
> +             if (!(features & flag)) {
> +                     features |= flag;
> +                     btrfs_set_super_incompat_flags(disk_super, features);
> +                     printk(KERN_INFO "btrfs: setting %llu feature flag\n",
> +                                      flag);

flag is u64, please use (unsigned long long)flag and possibly the new
btrfs_info replacement of printks.

> +             }
> +             spin_unlock(&fs_info->super_lock);
>       }
>  }

otherwise ok.

Reviewed-by: David Sterba <[email protected]>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to