Re: [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID rather than check EXT4_INODE_PROJINHERIT flag

Andreas Dilger
2021-12-17T00:17:32Z

[-- Attachment #1: Type: text/plain, Size: 2786 bytes --]

On Dec 13, 2021, at 10:06 PM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Dec 10, 2021 at 10:55:10PM +0300, Roman Anufriev wrote:
>>
>> On Tue, 7 Dec 2021, Theodore Y. Ts'o wrote:
>>
>>> On Tue, Dec 07, 2021 at 05:05:19PM +0300, Roman Anufriev wrote:
>>>>> Commit 7ddf79a10395 ("ext4: only set project inherit bit for directory")
>>>>> removes EXT4_INODE_PROJINHERIT flag from regular files. This makes
>>>>> ext4_statfs() output incorrect (function does not apply quota limits
>>>>> on used/available space, etc) when called on dentry of regular file
>>>>> with project quota enabled.
>>>
>>> Under what circumstance is userspace trying to call statfs on a file
>>> descriptor?
>>>
>>> Removing the test for EXT4_INODE_PROJINHERIT will cause
>>> incorrect/misleading results being returned in the case where we have
>>> a directory where a directory hierarchy is using project id's, but
>>> which is *not* using PROJINHERIT.
>>
>> I'm not sure I quite understood what will be wrong in that case, because
>> as Dave mentioned:
>>
>>> PROJINHERIT just indicates the default projid that an inode is
>>> created with; ...
>
> Directory inodes can have a project ID set without PROJINHERIT, it
> just means they are accounted to that specific project and have no
> special behaviour w.r.t. newly created children in the directory.
> i.e. without PROJINHERIT, all children will be created with a
> proj ID of zero rather than the projid of the parent directory.
>
> i.e. I can do `xfs_io -c "chproj -R 42" /mnt/test` and it will set
> all filesystem and directories to have a projid = 42, but
> PROJINHERIT is not set on any directory. The tree gets accounted to
> project 42, but it isn't a directory tree quota - it's just a user
> controlled aggregation of random files associated with the same
> project ID.
>
> Hence the statfs behaviour of "report project quota limits for
> directory tree" should only be triggered if PROJINHERIT is set on
> the directory, because that's the only viable indicator that
> directory tree quotas *may* be in use on the filesystem.

Sure, I think the question is if statfs() is called on a regular
file in a parent directory with PROJINHERIT set (which is easily
checked) should it return the project limits in the same way as
if statfs() is called on the directory itself?

It seems inconsistent for that statfs("/home/adilger/file") returns
full-filesystem information, but statfs("/home/adilger") and
statfs("/home/adilger/dir") would return project information, if
PROJINHERIT are set on "adilger/" and "dir/". It kind of ruins
the "tree" aspect, especially for processes that are in a container
that has limits on the subdirectory it is mounting.

Cheers, Andreas

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]