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 <dste...@suse.cz>
> --
> 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

Reply via email to