"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. Cheers, -- Luis