Re: [PATCH] btrfs: free qgroup configuration when quota enable fails
Qu Wenruo <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
在 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