All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Dilger <adilger@dilger.ca>
To: Dave Chinner <david@fromorbit.com>
Cc: Roman Anufriev <dotdot@yandex-team.ru>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	linux-ext4 <linux-ext4@vger.kernel.org>, Jan Kara <jack@suse.cz>,
	wshilong@ddn.com, Dmitry Monakhov <dmtrmonakhov@yandex-team.ru>,
	"Darrick J. Wong" <darrick.wong@oracle.com>
Subject: Re: [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID rather than check EXT4_INODE_PROJINHERIT flag
Date: Thu, 16 Dec 2021 17:17:28 -0700	[thread overview]
Message-ID: <A5DD4B7A-A3AE-4A00-943A-A35D98204764@dilger.ca> (raw)
In-Reply-To: <20211214050636.GE279368@dread.disaster.area>

[-- 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 --]

  reply	other threads:[~2021-12-17  0:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-07 13:18 [PATCH] ext4: compare inode's i_projid with EXT4_DEF_PROJID rather than check EXT4_INODE_PROJINHERIT flag Roman Anufriev
2021-12-07 14:05 ` Roman Anufriev
2021-12-07 19:34   ` Theodore Y. Ts'o
2021-12-09 22:53     ` Andreas Dilger
2021-12-09 23:30       ` Dave Chinner
2021-12-10 19:53         ` Roman Anufriev
2021-12-10 19:53       ` Roman Anufriev
2021-12-11  0:49       ` Darrick J. Wong
2021-12-10 19:55     ` Roman Anufriev
2021-12-14  5:06       ` Dave Chinner
2021-12-17  0:17         ` Andreas Dilger [this message]
2021-12-17  2:39           ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=A5DD4B7A-A3AE-4A00-943A-A35D98204764@dilger.ca \
    --to=adilger@dilger.ca \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=dmtrmonakhov@yandex-team.ru \
    --cc=dotdot@yandex-team.ru \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tytso@mit.edu \
    --cc=wshilong@ddn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.