Re: [RFC PATCH 2/2] ceph: quota: fix quota subdir mounts

From: Luis Henriques
Date: Thu Mar 07 2019 - 06:02:15 EST

"Yan, Zheng" <ukernel@xxxxxxxxx> writes:

> On Thu, Mar 7, 2019 at 2:21 AM Luis Henriques <lhenriques@xxxxxxxx> wrote:
>> "Yan, Zheng" <ukernel@xxxxxxxxx> writes:
>> > On Sat, Mar 2, 2019 at 3:13 AM Luis Henriques <lhenriques@xxxxxxxx> wrote:
>> >>
>> >> The CephFS kernel client doesn't enforce quotas that are set in a
>> >> directory that isn't visible in the mount point. For example, given the
>> >> path '/dir1/dir2', if quotas are set in 'dir1' and the mount is done in with
>> >>
>> >> mount -t ceph <server>:<port>:/dir1/ /mnt
>> >>
>> >> then the client can't access the 'dir1' inode from the quota realm dir2
>> >> belongs to.
>> >>
>> >> This patch fixes this by simply doing an MDS LOOKUPINO Op and grabbing a
>> >> reference to it (so that it doesn't disappear again). This also requires an
>> >> extra field in ceph_snap_realm so that we know we have to release that
>> >> reference when destroying the realm.
>> >>
>> >
>> > This may cause circle reference if somehow an inode owned by snaprealm
>> > get moved into mount subdir (other clients do rename). how about
>> > holding these inodes in mds_client?
>> Ok, before proceeded any further I wanted to make sure that what you
>> were suggesting was something like the patch below. It simply keeps a
>> list of inodes in ceph_mds_client until the filesystem is umounted,
>> iput()ing them at that point.
> yes,
>> I'm sure I'm missing another place where the reference should be
>> dropped, but I couldn't figure it out yet. It can't be
>> ceph_destroy_inode; drop_inode_snap_realm is a possibility, but what if
>> the inode becomes visible in the meantime? Well, I'll continue thinking
>> about it.
> why do you think we need to clean up the references at other place.
> what problem you encountered.

I'm not really seeing any issue, at least not at the moment. I believe
that we could just be holding refs to inodes that may not exist anymore
in the cluster. For example, in client 1:

mkdir -p /mnt/a/b
setfattr -n ceph.quota.max_files -v 5 /mnt/a

In client 2 we mount:

mount <addr>:/a/b /mnt

This client will access the realm and inode for 'a' (adding that inode
to the ceph_mds_client list), because it has quotas. If client 1 then
deletes 'a', client 2 will continue to have a reference to that inode in
that list. That's why I thought we should be able to clean up that refs
list in some other place, although that's probably a big deal, since we
won't be able to a lot with this mount anyway.