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

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.

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.

Thanks,
Guanjie