Re: dcache: remove trylock loops (was Re: [BUG] lock_parent() breakage when used from shrink_dentry_list())

From: Al Viro
Date: Mon Mar 12 2018 - 16:33:54 EST


On Mon, Mar 12, 2018 at 08:05:41PM +0000, Al Viro wrote:

> Does any of the d_find_alias() callers want those dentries? That is,
> non-directories from d_obtain_alias() still not attached to a parent;
> note that exportfs_decode_fh() is *not* one of such places - we don't
> use d_find_alias() there at all. If there's no such caller, we can
> bloody well just drop the discon_alias logics and be done with that;
> if there is, that commit has introduced a bug. I might have missed
> a part of threads related to that patch, so my apologies if it had
> been discussed.
>
> Neil, what's the situation there? A lot of those callers clearly treat
> the "only disconnected IS_ROOT alias exist" same as "no aliases at all";
> it looks like the change might have been the right thing, but it sure
> as hell shouldn't be an undocumented side effect...

FWIW, callers are
drivers/staging/lustre/lustre/llite/llite_lib.c:2448: dentry = d_find_alias(page->mapping->host);
definitely doesn't want them
fs/ceph/caps.c:2945: while ((dn = d_find_alias(inode))) {
fs/ceph/mds_client.c:1930: dentry = d_find_alias(inode);
ditto - it's building a pathname.
fs/ceph/mds_client.c:2910: dentry = d_find_alias(inode);
ditto.
fs/ceph/mds_client.c:3374: parent = d_find_alias(inode);
clearly at least expects a directory (feeds to d_lookup(), etc.)
fs/coda/cache.c:112: alias_de = d_find_alias(inode);
directory-only
fs/fat/namei_vfat.c:736: alias = d_find_alias(inode);
we will reject any IS_ROOT there anyway
fs/fs-writeback.c:2071: dentry = d_find_alias(inode);
wants a name, these don't have one.
fs/fuse/dir.c:966: dir = d_find_alias(parent);
directory-only
fs/hostfs/hostfs_kern.c:129: dentry = d_find_alias(ino);
wants a pathname
fs/nilfs2/super.c:931: dentry = d_find_alias(inode);
directory-only
fs/nilfs2/super.c:1025: dentry = d_find_alias(inode);
bloody better be a directory (root one, at that)
fs/xfs/xfs_filestream.c:293: dentry = d_find_alias(inode);
no parent, no love.
mm/shmem.c:3435: dentry = d_find_alias(inode);
no d_obtain_alias() on that one
security/commoncap.c:391: dentry = d_find_alias(inode);
security/lsm_audit.c:295: dentry = d_find_alias(inode);
wants a name
security/selinux/hooks.c:1536: dentry = d_find_alias(inode);
security/selinux/hooks.c:1646: dentry = d_find_alias(inode);

So all but four of them clearly do *not* want those suckers. And remaining
ones are in
* ceph invalidate_aliases()
* cap_inode_getsecurity()
* selinux inode_doinit_with_dentry() (two call sites, very alike)
Do any of those want that stuff?