On thu, 18 Apr 2013 00:17:11 +0200, David Sterba wrote:
> 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.
Yes, I want to add more items from super block under this lock.
>
>> @@ -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.
OK, I'll modify my patch.
Thanks for your view.
Miao
>
>> + }
>> + spin_unlock(&fs_info->super_lock);
>> }
>> }
>
> otherwise ok.
>
> Reviewed-by: David Sterba <dsterba@xxxxxxx>
> --
> 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
>
--
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
