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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, 9 Dec 2021, Andreas Dilger wrote:

On Dec 7, 2021, at 12:34 PM, Theodore Y. Ts'o <tytso@xxxxxxx> 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?

This is exactly my reasoning for this patch.

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.

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:

#ifdef CONFIG_QUOTA
-	if (ext4_test_inode_flag(dentry->d_inode, EXT4_INODE_PROJINHERIT) &&
+	if (ext4_test_inode_flag(S_ISDIR(dentry->d_inode) ? dentry->d_inode :
+			   dentry->d_parent->d_inode, EXT4_INODE_PROJINHERIT) &&
	    sb_has_quota_limits_enabled(sb, PRJQUOTA))
		ext4_statfs_project(sb, EXT4_I(dentry->d_inode)->i_projid, buf);
#endif

Roman, does that work for you?

Yes, it was actually the first thing that came to my mind. But later I
realised, that there may be some pitfalls and it would probably make more
sense to check inode's own project id and report stats based on that. As
I thought that we check presense of EXT4_INODE_PROJINHERIT flag only to
make sure that this inode belongs to some project.

								Roman



[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux