Re: [PATCH v4] kernfs: allocate inodes and dentry outside kernfs_rwsem

[Chris Down <chris@chrisdown.name>]

Hi Shakeel,

Shakeel Butt writes:
>+	new = new_inode(sb);
>+	if (!new)
>+		return ERR_PTR(-ENOMEM);

Not a big deal, but one thing changed from v3 that the changelog doesn't
mention. Without an ilookup() first, every caller now needs the
preallocated inode even when one is already hashed. Before this patch
iget_locked() returned a hashed inode without allocating. So when
new_inode() fails, decoding a handle for a live node whose inode is
cached now fails with -ENOMEM, and a stale handle gets -ENOMEM instead of
-ESTALE. kernfs_iop_lookup() does the same for a node that already has an
inode.

 From a quick check I did with failslab limited to inode_cache and an fd
held open on a cgroup2 directory, open_by_handle_at() on its handle
succeeds without this patch and fails with ENOMEM with it.

Now, new_inode() only fails there for an OOM victim, or in a memcg whose
OOM killer is disabled or has nothing left to kill, and those tasks are
failing most other allocations too, so I don't think it's worth
complicating kernfs_get_inode_locked() for. But it is a second case
besides an inactive node where an ilookup() first changes the result.

Not a problem, just an observation, so feel free to add:

Reviewed-by: Chris Down <chris@chrisdown.name>

Thanks!

Chris