Re: [PATCH v3 1/2] fs: use fake_file container for internal files with fake f_path

[Christoph Hellwig <hch@infradead.org>]

On Sun, Jun 11, 2023 at 10:47:05PM +0300, Amir Goldstein wrote:
> Overlayfs and cachefiles use open_with_fake_path() to allocate internal
> files, where overlayfs also puts a "fake" path in f_path - a path which
> is not on the same fs as f_inode.

But cachefs doesn't, so this needs a better explanation / documentation.

> Allocate a container struct file_fake for those internal files, that
> is used to hold the fake path along with an optional real path.

The idea looks sensible, but fake a is a really weird term here.
I know open_with_fake_path also uses it, but we really need to
come up with a better name, and also good documentation of the
concept here.

> +/* Returns the real_path field that could be empty */
> +struct path *__f_real_path(struct file *f)
> +{
> +	struct file_fake *ff = file_fake(f);
> +
> +	if (f->f_mode & FMODE_FAKE_PATH)
> +		return &ff->real_path;
> +	else
> +		return &f->f_path;
> +}

two of the three callers always have FMODE_FAKE_PATH set, so please
just drop this helper and open code it in the three callers.

> +
> +/* Returns the real_path if not empty or f_path */
> +const struct path *f_real_path(struct file *f)
> +{
> +	const struct path *path = __f_real_path(f);
> +
> +	return path->dentry ? path : &f->f_path;
> +}
> +EXPORT_SYMBOL(f_real_path);

This is only needed by the few places like nfsd or btrfs send
that directlycall fsnotify and should at very least be
EXPORT_SYMBOL_GPL.  But I suspect with all the exta code, fsnotify_file
really should move out of line and have an EXORT_SYMBOL_GPL instead.

> +
> +const struct path *f_fake_path(struct file *f)
> +{
> +	return &f->f_path;
> +}
> +EXPORT_SYMBOL(f_fake_path);

.. and this helper is completely pointless.

> +extern struct file *alloc_empty_file(int flags, const struct cred *cred);
> +extern struct file *alloc_empty_file_fake(int flags, const struct cred *cred);
> +extern struct path *__f_real_path(struct file *f);

Please drop all the pointless externs while you're at it.