On Wed, Jun 12, 2024 at 2:40 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > On Fri, Jun 07, 2024 at 09:17:34AM +0300, Amir Goldstein wrote: > > On Thu, Jun 6, 2024 at 5:27 AM Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > > > > > On Wed, Jun 05, 2024 at 08:13:15AM +0300, Amir Goldstein wrote: > > > > On Wed, Jun 5, 2024 at 3:38 AM Darrick J. Wong <djwong@xxxxxxxxxx> wrote: > > > > > On Tue, Jun 04, 2024 at 10:58:43AM +0200, Jan Kara wrote: > > > > > > On Mon 03-06-24 10:42:59, Darrick J. Wong wrote: > > > > > > > I do -- allowing unpriviledged users to create symlinks that consume > > > > > > > icount (and possibly bcount) in the root project breaks the entire > > > > > > > enforcement mechanism. That's not the way that project quota has worked > > > > > > > on xfs and it would be quite rude to nullify the PROJINHERIT flag bit > > > > > > > only for these special cases. > > > > > > > > > > > > OK, fair enough. I though someone will hate this. I'd just like to > > > > > > understand one thing: Owner of the inode can change the project ID to 0 > > > > > > anyway so project quotas are more like a cooperative space tracking scheme > > > > > > anyway. If you want to escape it, you can. So what are you exactly worried > > > > > > about? Is it the container usecase where from within the user namespace you > > > > > > cannot change project IDs? > > > > > > > > > > Yep. > > > > > > > > > > > Anyway I just wanted to have an explicit decision that the simple solution > > > > > > is not good enough before we go the more complex route ;). > > > > > > > > > > Also, every now and then someone comes along and half-proposes making it > > > > > so that non-root cannot change project ids anymore. Maybe some day that > > > > > will succeed. > > > > > > > > > > > > > I'd just like to point out that the purpose of the project quotas feature > > > > as I understand it, is to apply quotas to subtrees, where container storage > > > > is a very common private case of project subtree. > > > > > > That is the most modern use case, yes. > > > > > > [ And for a walk down history lane.... ] > > > > > > > The purpose is NOT to create a "project" of random files in random > > > > paths. > > > > > > This is *exactly* the original use case that project quotas were > > > designed for back on Irix in the early 1990s and is the original > > > behaviour project quotas brought to Linux. > > > > > > Project quota inheritance didn't come along until 2005: > > > > > > commit 65f1866a3a8e512d43795c116bfef262e703b789 > > > Author: Nathan Scott <nathans@xxxxxxx> > > > Date: Fri Jun 3 06:04:22 2005 +0000 > > > > > > Add support for project quota inheritance, a merge of Glens changes. > > > Merge of xfs-linux-melb:xfs-kern:22806a by kenmcd. > > > > > > And full support for directory tree quotas using project IDs wasn't > > > fully introduced until a year later in 2006: > > > > > > commit 4aef4de4d04bcc36a1461c100eb940c162fd5ee6 > > > Author: Nathan Scott <nathans@xxxxxxx> > > > Date: Tue May 30 15:54:53 2006 +0000 > > > > > > statvfs component of directory/project quota support, code originally by Glen. > > > Merge of xfs-linux-melb:xfs-kern:26105a by kenmcd. > > > > > > These changes were largely done for an SGI NAS product that allowed > > > us to create one great big XFS filesystem and then create > > > arbitrarily sized, thin provisoned "NFS volumes" as directory > > > quota controlled subdirs instantenously. The directory tree quota > > > defined the size of the volume, and so we could also grow and shrink > > > them instantenously, too. And we could remove them instantenously > > > via background garbage collection after the export was removed and > > > the user had been told it had been destroyed. > > > > > > So that was the original use case for directory tree quotas on XFS - > > > providing scalable, fast management of "thin" storage for a NAS > > > product. Projects quotas had been used for accounting random > > > colections of files for over a decade before this directory quota > > > construct was created, and the "modern" container use cases for > > > directory quotas didn't come along until almost a decade after this > > > capability was added. > > > > > > > Cool. Didn't know all of this. > > Lucky for us, those historic use cases are well distinguished from > > the modern subtree use case by the opt-in PROJINHERIT bit. > > So as long as PROJINHERIT is set, my assumptions mostly hold(?) > > I'm not sure what assumptions you are making, so I can't really > make any binding statement on this except to say that the existance > of PROJINHERIT on directories does not mean strict directory quotas > are being used. > > i.e. PROJINHERIT is just a flag to automatically tag inodes with > aproject ID on creation. It -can- be used as a directory tree quota > if specific restrictions in the use of the projinherit flag are > enforced, but it can also be used for the original project ID use > case so users don't have to manually tag files they create or move > to a projet directory with the right project ID after the fact. > > IOWs, directories that have the same projid and PROJINHERIT set > do not need to be in a single hierarchy, and hence do not fit the > definition of a directory tree quota. e.g: > > /projects/docs/project1 > /projects/docs/project2 > /projects/docs/project3 > /projects/src/project1 > /projects/src/project2 > /projects/src/project3 > /projects/build/project1 > /projects/build/project2 > /projects/build/project3 > ..... > /home/user1/project1 > /home/user2/project3 > /home/user6/project2 > > Now we have multiple different disjoint directory heirarchies with > the same project ID because they contain files belonging to a > specific project. This is still a random collection of files in > random paths that are accounted to a project ID, but it's also using > PROJINHERIT to assign the default project ID to files created in the > respective project directories. > > The kernel has no idea that a project ID is associated with a single > directory tree. The kernel quota accounting itself simple sees > project IDs on inodes and accounts to that project ID. The create > and rename code simply see the parent PROJINHERIT bit and so need to > set up the new directory entry to be accounted to that different > project ID. It's all just mechanism in the kernel - applying project > quotas to enforce directory tree quotas is entirely a userspace > administration constraint.... > > The only thing we do to help userspace administration is restrict > changing project IDs to the init namespace, thereby preventing > containers from being able to manipulate project IDs. This allows > the init namespace to set the policy for project quota use and hence > allow it to be used for directory tree quotas for container space > management. This is essentially an extension of the original NAS > use case, in that NFS exports were the isolation barrier that hid > the underlying filesystem structure and project ID quota usage > from the end users.... > > > > > My point is that changing the project id of a non-dir child to be different > > > > from the project id of its parent is a pretty rare use case (I think?). > > > > > > Not if you are using project quotas as they were originally intended > > > to be used. > > > > > > > Rephrase then: > > > > Changing the projid of a non-dir child to be different from the projid > > of its parent, which has PROJINHERIT bit set, is a pretty rare use case(?) > > I have no data to indicate how rare it might be - the kernel has > never enforced a policy that disallows changing project IDs when > PROJINHERIT is set and any user with write permission can change > project IDs even when PROJINHERIT is set. Hence we have to assume > that there are people out there that rely on this behaviour, > regardless of how rare it is... > > > > > If changing the projid of non-dir is needed for moving it to a > > > > different subtree, > > > > we could allow renameat2(2) of non-dir with no hardlinks to implicitly > > > > change its > > > > inherited project id or explicitly with a flag for a hardlink, e.g.: > > > > renameat2(olddirfd, name, newdirfd, name, RENAME_NEW_PROJID). > > > > > > Why? > > > > > > The only reason XFS returns -EXDEV to rename across project IDs is > > > because nobody wanted to spend the time to work out how to do the > > > quota accounting of the metadata changed in the rename operation > > > accurately. So for that rare case (not something that would happen > > > on the NAS product) we returned -EXDEV to trigger the mv command to > > > copy the file to the destination and then unlink the source instead, > > > thereby handling all the quota accounting correctly. > > > > > > IOWs, this whole "-EXDEV on rename across parent project quota > > > boundaries" is an implementation detail and nothing more. > > > Filesystems that implement project quotas and the directory tree > > > sub-variant don't need to behave like this if they can accurately > > > account for the quota ID changes during an atomic rename operation. > > > If that's too hard, then the fallback is to return -EXDEV and let > > > userspace do it the slow way which will always acocunt the resource > > > usage correctly to the individual projects. > > > > > > Hence I think we should just fix the XFS kernel behaviour to do the > > > right thing in this special file case rather than return -EXDEV and > > > then forget about the rest of it. Sure, update xfs_repair to fix the > > > special file project id issue if it trips over it, but other than > > > that I don't think we need anything more. If fixing it requires new > > > syscalls and tools, then that's much harder to backport to old > > > kernels and distros than just backporting a couple of small XFS > > > kernel patches... > > > > > > > I assume that by "fix the XFS behavior" you mean > > "we could allow renameat2(2) of non-dir with no hardlinks to implicitly > > change its inherited project id"? > > (in case the new parent has the PROJINHERIT bit) > > so that the RENAME_NEW_PROJID behavior would be implicit. > > No, I meant "fix the original hardlink of an inode with no project ID > within a PROJINHERIT directory always gets -EXDEV" by allowing > hard links when the source file has no project ID specified. > > Actually, looking at 6.10-rc3, this has been merged already. So IMO > there's nothing more we need to do here. > I don't see it. Which commit is that? I agree that would be enough to address the original report. > > Unlike rename() from one parent to the other, link()+unlink() > > is less obvious. > > > > The "modern" use cases that I listed where implicit change of projid > > does not suffice are: > > > > 1. Share some inodes (as hardlinks) among projects > > 2. Recursively changing a subtree projid > > > > They could be implemented by explicit flags to renameat2()/linkat() and > > they could be implemented by [gs]etfsxattrat(2) syscalls. > > What are you expecting to happen when you hardlink an inode across > multiple PROJINHERIT directories? I mean, an inode can only have > one project ID, so you can't link it into mulitple projects at once > if you are using PROJINHERIT (xfs_link() expressly forbids it). > > So I'm really not sure what problem you are trying to describe or > solve here. Can you elaborate more? The problem that Andrey described started with a subtree that has no projid and then projid was applied to all non-special files. The suggested change to relax hardlink of projid 0 solves this case. But a subtree could also be changed by user, say from projid P1 to projid P2 and in that case inherited projid P1 will remain on the special files, causing a similar failure to link() without the projid 0 relax rule. Anyway, this discussion has wandered off too far from the original problem. If the simple fix is enough for Andrey, it's fine by me. If other people are interested in syscall or other means to change fsxattr on special files, it's fine by me as well. Thanks, Amir.
