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