Re: [PATCH] btrfs: free qgroup configuration when quota enable fails

[Qu Wenruo <wqu@suse.com>]

在 2026/8/14 22:00, Guanjie Chen 写道:
> Hi Qu,
> 
>> We can reach this out branch if btrfs_start_transaction() failed, but
>> the fs_info->quota_root is already setup by another process.
> 
> Do you mean the window where qgroup_ioctl_lock is dropped before
> btrfs_start_transaction()?
> 
> I don't think another task can set fs_info->quota_root in that window.
> The quota ioctl holds fs_info->subvol_sem for writing across the whole
> btrfs_quota_enable() call, including while qgroup_ioctl_lock is dropped.
> 
> As far as I can see, quota_root is only set in two places:
> btrfs_quota_enable(), which is serialized by subvol_sem, and
> btrfs_read_roots(), which runs during mount initialization.

Right, I missed that rwsem.

But still, for error handling one should be in the reverse order.

And for out: label, there are cases we're going out label without 
creating any qgroup. In that case, qgroups removing should be done only 
for cases where a qgroup is created.

> 
> The disable path also calls btrfs_free_qgroup_config() and relies on the
> same subvol_sem and qgroup_ioctl_lock serialization for the qgroup
> configuration. If subvol_sem did not prevent another quota operation
> here, the disable path would have a similar lifetime problem.
> 
> So if quota_root was NULL at the first check, I believe it must still be
> NULL when btrfs_start_transaction() fails. I agree that the changelog
> should have mentioned subvol_sem as part of the protection.
> 
> Did I miss another runtime path that can set quota_root without holding
> subvol_sem?
> 
> Also, just to clarify, this is not my first btrfs contribution, although
> my previous change was only a small one.

The same, LLM disclosure if you're using it.

Nowadays I won't trust anyone who is not a regular contributor.

> 
> Thanks,
> Guanjie