Message-ID: <20211209233017.GA279368@dread.disaster.area>
Date:   Fri, 10 Dec 2021 10:30:17 +1100
From:   Dave Chinner <david@...morbit.com>
To:     Andreas Dilger <adilger@...ger.ca>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>,
        Roman Anufriev <dotdot@...dex-team.ru>,
        linux-ext4 <linux-ext4@...r.kernel.org>, Jan Kara <jack@...e.cz>,
        Wang Shilong <wangshilong1991@...il.com>,
        Dmitry Monakhov <dmtrmonakhov@...dex-team.ru>,
        "Darrick J. Wong" <darrick.wong@...cle.com>
Subject: Re: [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID
 rather than check EXT4_INODE_PROJINHERIT flag

On Thu, Dec 09, 2021 at 03:53:55PM -0700, Andreas Dilger wrote:
> On Dec 7, 2021, at 12:34 PM, Theodore Y. Ts'o <tytso@....EDU> 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?
> 
> Who knows what users do?  Calling statfs() on a regular file works fine
> (returns stats for the filesystem), so I don't see why it wouldn't be
> consistent when calling statfs() on a file with projid set?
> 
> Darrick, how does XFS handle this case?  I think it makes sense to be
> consistent with that implementation, since that was the main reason to
> remove PROJINHERIT from regular files in the first place.

If PROJINHERIT is set on the inode, it will return the information
for the projid on that inode. XFS doesn't care what type of inode it
is, just whether the PROJINHERIT flag is set.

That said, on XFS, only directory inodes will have the PROJINHERIT
flag set. So, in effect, only statfs() on directory inodes can
report project quota limits.

PROJINHERIT just indicates the default projid that an inode is
created with; it does not mean that directory tree quotas are what
the user it doing with them...

> > 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.
> 
> One alternative would be to check the PROJINHERIT status of the parent
> directory after calling statfs() on the regular file?  That should
> keep the semantics for PROJINHERIT the same, but avoid inconsistent
> results if called on a regular file:

This just opens a bigger can of worms that still has no consistent
solution.

What if the user has changed the projid of the file and it doesn't
match the parent directory? That then reports something irrelevant
to the user.

What if there are hard links and the parent directories have
different projid state? This can happen - we don't allow hard links
into a new projid controlled directory, but we allow them into
non-projid controlled directories even if the source is from a
projid controlled heirarchy. We can add PROJINHERIT after a
directory has already been populated. We can remove PROJINHERIT,
too, after hardlinks within the same projid have been created. Hence
a regular file inode can have different parent PROJINHERIT depending
on path.  How do you do consistency then, because it's clearly not a
directory quota controlled setup and there's no way of detecting
that from statfs() context?

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com

Powered by blists - more mailing lists