Re: [PATCH 19/21] readdir: take no inode lock on an immutable directory

[NeilBrown]

On Fri, 02 Oct 2026, Christian Brauner wrote:
> iterate_dir() takes the directory's i_rwsem shared and holds it across
> ->iterate_shared(). For a directory that never has an entry and is
> never removed the lock keeps nothing still, it only orders every reader
> and every writer of that inode behind each other.
> 
> For the directory of a nullfs instance that matters. The instance of
> the initial mount namespace is the root of every empty mount namespace
> and the private instance is the root of every kernel thread, so one
> inode is shared across users who have nothing else in common. And a
> reader can hold the lock for as long as it likes: back the getdents()
> buffer with a mapping of a file on a FUSE mount of your own, let the
> copy of "." and ".." fault and let the server wait. Queue an exclusive
> taker behind it, a mkdir() in that directory goes through start_dirop()
> before the read-only mount is reported, and from then on every lookup
> that misses the dcache in that directory, every create and every mount
> on it waits until the server answers. One user of an empty mount
> namespace stalls all the others.
> 
> Add FOP_IMMUTABLE for the file operations of a directory that never
> changes and is never removed and let iterate_dir() skip the lock for
> it. The flag never changes for a file, ->f_pos is protected by
> f_pos_lock since directories are FMODE_ATOMIC_POS, IS_DEADDIR can't be
> set on such a directory and neither touch_atime() nor fsnotify take
> i_rwsem. Set it on the nullfs directory. The placeholder directories of
> libfs never have an entry either but their owners remove them, so they
> keep the lock.

It isn't strictly necessary that the directory never changes.  It is
only necessary that the filesystem doesn't depend of i_rwsem for
exclusion between iterate_dir() and directory changes, either because
there are no changes, or because it uses internal locking.

This flag would be good for procfs - those directories that need to
d_alloc_parallel() any name they find that isn't already is dcache.
My current approach is to drop the lock and reclaim it after
d_alloc_parallel().  With FMODE_ATOMIC_POS set there would be nothing
left to fix.

Reviewed-by: NeilBrown <neil@xxxxxxxxxx>

Thanks,
NeilBrown


> 
> Fixes: 9d4e752a24f7 ("namespace: allow creating empty mount namespaces")
> Cc: stable@xxxxxxxxxxxxxxx # v7.1+
> Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>
> ---
>  fs/nullfs.c        |  1 +
>  fs/readdir.c       | 13 +++++++++----
>  include/linux/fs.h |  2 ++
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nullfs.c b/fs/nullfs.c
> index f76b87cf1841..bfc04bca3940 100644
> --- a/fs/nullfs.c
> +++ b/fs/nullfs.c
> @@ -44,6 +44,7 @@ static const struct file_operations nullfs_dir_operations = {
>  	.lock		= nullfs_nolock,
>  	.flock		= nullfs_nolock,
>  	.setlease	= nullfs_nolease,
> +	.fop_flags	= FOP_IMMUTABLE,
>  };
>  
>  static int nullfs_fs_fill_super(struct super_block *s, struct fs_context *fc)
> diff --git a/fs/readdir.c b/fs/readdir.c
> index 76bb1ae3a450..f2288841337a 100644
> --- a/fs/readdir.c
> +++ b/fs/readdir.c
> @@ -87,6 +87,8 @@ EXPORT_SYMBOL(wrap_directory_iterator);
>  int iterate_dir(struct file *file, struct dir_context *ctx)
>  {
>  	struct inode *inode = file_inode(file);
> +	/* never an entry, never removed: nothing for the lock to keep still */
> +	bool locked = !(file->f_op->fop_flags & FOP_IMMUTABLE);
>  	int res = -ENOTDIR;
>  
>  	if (!file->f_op->iterate_shared)
> @@ -100,9 +102,11 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
>  	if (res)
>  		goto out;
>  
> -	res = down_read_killable(&inode->i_rwsem);
> -	if (res)
> -		goto out;
> +	if (locked) {
> +		res = down_read_killable(&inode->i_rwsem);
> +		if (res)
> +			goto out;
> +	}
>  
>  	res = -ENOENT;
>  	if (!IS_DEADDIR(inode)) {
> @@ -112,7 +116,8 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
>  		fsnotify_access(file);
>  		file_accessed(file);
>  	}
> -	inode_unlock_shared(inode);
> +	if (locked)
> +		inode_unlock_shared(inode);
>  out:
>  	return res;
>  }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 784fa20217c4..deb411e86661 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1978,6 +1978,8 @@ struct file_operations {
>  #define FOP_ASYNC_LOCK		((__force fop_flags_t)(1 << 6))
>  /* File system supports uncached read/write buffered IO */
>  #define FOP_DONTCACHE		((__force fop_flags_t)(1 << 7))
> +/* Never changes and is never removed, readdir of a directory takes no lock */
> +#define FOP_IMMUTABLE		((__force fop_flags_t)(1 << 8))
>  
>  /* Wrap a directory iterator that needs exclusive inode access */
>  int wrap_directory_iterator(struct file *, struct dir_context *,
> 
> -- 
> 2.53.0
> 
>