Re: [PATCH v5 4/5] fs: use backing_file container for internal files with "fake" f_path

[Christoph Hellwig <hch@lst.de>]

> -	if (!(f->f_mode & FMODE_NOACCOUNT))
> +	if (unlikely(f->f_mode & FMODE_BACKING))
> +		path_put(backing_file_real_path(f));
> +	else
>  		percpu_counter_dec(&nr_files);

This is still missing the earlier pointed out fix that we still need
the FMODE_NOACCOUNT check, isn't it?

> + * This is only for kernel internal use, and the allocate file must not be
> + * installed into file tables or such.

I'd use the same blurb I'd suggest for the previous patch here as well.

> +/**
> + * backing_file_open - open a backing file for kernel internal use
> + * @path:	path of the file to open
> + * @flags:	open flags
> + * @path:	path of the backing file
> + * @cred:	credentials for open
> + *
> + * Open a file whose f_inode != d_inode(f_path.dentry).
> + * the returned struct file is embedded inside a struct backing_file
> + * container which is used to store the @real_path of the inode.
> + * The @real_path can be accessed by backing_file_real_path() and the
> + * real dentry can be accessed with file_dentry().
> + * The kernel internal backing file is not accounted in nr_files.
> + * This is only for kernel internal use, and must not be installed into
> + * file tables or such.
> + */

I still find this comment not very descriptive.  Here is my counter
suggestion, which I'm also not totally happy with, and which might not
even be factually correct as I'm trying to understand the use case a bit
better by reading the code:

 * Open a backing file for a stackable file system (e.g. overlayfs).
 * For these backing files that reside on the underlying file system, we still
 * want to be able to return the path of the upper file in the stackable file
 * system.  This is done by embedding the returned file into a container
 * structure that also stores the path on the upper file system, which can be
 * retreived using backing_file_real_path().
 *
 * The return file is not accounted in nr_files and must not be installed
 * into the file descriptor table.

> +static inline const struct path *f_real_path(struct file *f)

Question from an earlier revision: why f_real_path and not file_real_path?

Also this really needs a kerneldoc comment explaining when it should
be used.