On Thu, May 07, 2020 at 06:48:24AM +0800, Qu Wenruo wrote: > >>> @@ -2809,6 +2821,8 @@ int btrfs_qgroup_inherit(struct btrfs_trans_handle *trans, u64 srcid, > >>> out: > >>> if (!committing) > >>> mutex_unlock(&fs_info->qgroup_ioctl_lock); > >>> + if (need_rescan) > >>> + fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT; > > > > This got me curious, a non-atomic change to qgroup_flags and without > > any protection. The function is not running in a safe context (like > > quota enable or disable) so lack of synchronization seems suspicious. I > > grepped for other changes to the qgroup_flags and it's very > > inconsistent. Sometimes it's the fs_info::qgroup_lock, no lokcing at all > > or no obvious lock but likely fs_info::qgroup_ioctl_lock or > > qgroup_rescan_lock. > > > > I was considering using atomic bit updates but that would be another > > patch. > > > Isn't the context safe because we're at the commit transaction critical > section? I don't see why, the qgroup_flags could be changed from ioctls, eg. adding a group relation and other places. Without protection the update can race and some of the changes lost.
