Re: [PATCH 2/2] Btrfs: use a lock to protect incompat/compat flag of the super block

[Miao Xie <miaox@cn.fujitsu.com>]

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@suse.cz>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>