[PATCH] vfs: change 'struct file *' argument to 'const struct file *' where possible

[PATCH] vfs: change 'struct file *' argument to 'const struct file *' where possible

[Miklos Szeredi]

Let file_user_path(), file_user_inode() and file_clone_open() take const
pointer.

This allows removing a cast in ovl_open_realfile() when calling
backing_file_open().

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---

This depends on commit 924577e4f6ca ("ovl: Fix nested backing file paths") in

  git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs.git ovl-fixes

 fs/file_table.c     | 10 ++++++----
 fs/internal.h       |  1 +
 fs/overlayfs/file.c |  2 +-
 include/linux/fs.h  | 12 ++++++------
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/file_table.c b/fs/file_table.c
index c04ed94cdc4b..3b3f920a9175 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -52,16 +52,18 @@ struct backing_file {
 	};
 };
 
-static inline struct backing_file *backing_file(struct file *f)
+#define backing_file(f) container_of(f, struct backing_file, file)
+
+struct path *backing_file_user_path(struct file *f)
 {
-	return container_of(f, struct backing_file, file);
+	return &backing_file(f)->user_path;
 }
 
-struct path *backing_file_user_path(struct file *f)
+const struct path *backing_file_user_path_c(const struct file *f)
 {
 	return &backing_file(f)->user_path;
 }
-EXPORT_SYMBOL_GPL(backing_file_user_path);
+EXPORT_SYMBOL_GPL(backing_file_user_path_c);
 
 static inline void file_free(struct file *f)
 {
diff --git a/fs/internal.h b/fs/internal.h
index b9b3e29a73fd..84a330763343 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -100,6 +100,7 @@ extern void chroot_fs_refs(const struct path *, const struct path *);
 struct file *alloc_empty_file(int flags, const struct cred *cred);
 struct file *alloc_empty_file_noaccount(int flags, const struct cred *cred);
 struct file *alloc_empty_backing_file(int flags, const struct cred *cred);
+struct path *backing_file_user_path(struct file *f);
 
 static inline void file_put_write_access(struct file *file)
 {
diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index dfea7bd800cb..f5b8877d5fe2 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -48,7 +48,7 @@ static struct file *ovl_open_realfile(const struct file *file,
 		if (!inode_owner_or_capable(real_idmap, realinode))
 			flags &= ~O_NOATIME;
 
-		realfile = backing_file_open(file_user_path((struct file *) file),
+		realfile = backing_file_open(file_user_path(file),
 					     flags, realpath, current_cred());
 	}
 	ovl_revert_creds(old_cred);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 016b0fe1536e..7db9cd5a4bee 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2813,7 +2813,7 @@ struct file *dentry_open_nonotify(const struct path *path, int flags,
 				  const struct cred *cred);
 struct file *dentry_create(const struct path *path, int flags, umode_t mode,
 			   const struct cred *cred);
-struct path *backing_file_user_path(struct file *f);
+const struct path *backing_file_user_path_c(const struct file *f);
 
 /*
  * When mmapping a file on a stackable filesystem (e.g., overlayfs), the file
@@ -2825,21 +2825,21 @@ struct path *backing_file_user_path(struct file *f);
  * by fstat() on that same fd.
  */
 /* Get the path to display in /proc/<pid>/maps */
-static inline const struct path *file_user_path(struct file *f)
+static inline const struct path *file_user_path(const struct file *f)
 {
 	if (unlikely(f->f_mode & FMODE_BACKING))
-		return backing_file_user_path(f);
+		return backing_file_user_path_c(f);
 	return &f->f_path;
 }
 /* Get the inode whose inode number to display in /proc/<pid>/maps */
-static inline const struct inode *file_user_inode(struct file *f)
+static inline const struct inode *file_user_inode(const struct file *f)
 {
 	if (unlikely(f->f_mode & FMODE_BACKING))
-		return d_inode(backing_file_user_path(f)->dentry);
+		return d_inode(backing_file_user_path_c(f)->dentry);
 	return file_inode(f);
 }
 
-static inline struct file *file_clone_open(struct file *file)
+static inline struct file *file_clone_open(const struct file *file)
 {
 	return dentry_open(&file->f_path, file->f_flags, file->f_cred);
 }
-- 
2.49.0

Re: [PATCH] vfs: change 'struct file *' argument to 'const struct file *' where possible

[Matthew Wilcox]

On Mon, May 05, 2025 at 05:41:47PM +0200, Miklos Szeredi wrote:
> -static inline struct backing_file *backing_file(struct file *f)
> +#define backing_file(f) container_of(f, struct backing_file, file)
> +
> +struct path *backing_file_user_path(struct file *f)
>  {
> -	return container_of(f, struct backing_file, file);
> +	return &backing_file(f)->user_path;
>  }
>  
> -struct path *backing_file_user_path(struct file *f)
> +const struct path *backing_file_user_path_c(const struct file *f)
>  {
>  	return &backing_file(f)->user_path;
>  }
> -EXPORT_SYMBOL_GPL(backing_file_user_path);
> +EXPORT_SYMBOL_GPL(backing_file_user_path_c);

May I suggest:

#define backing_file_user_path(f)	(_Generic((f),		\
	const struct file *:	(const struct path *)&backing_file(f)->user_path,  \
	struct file *:		(struct path *)&backing_file(f)->user_path)))

It's the same technique we use for page_folio() so it's quite well
tested.

Re: [PATCH] vfs: change 'struct file *' argument to 'const struct file *' where possible

[Al Viro]

On Mon, May 05, 2025 at 05:27:34PM +0100, Matthew Wilcox wrote:
> On Mon, May 05, 2025 at 05:41:47PM +0200, Miklos Szeredi wrote:
> > -static inline struct backing_file *backing_file(struct file *f)
> > +#define backing_file(f) container_of(f, struct backing_file, file)
> > +
> > +struct path *backing_file_user_path(struct file *f)
> >  {
> > -	return container_of(f, struct backing_file, file);
> > +	return &backing_file(f)->user_path;
> >  }
> >  
> > -struct path *backing_file_user_path(struct file *f)
> > +const struct path *backing_file_user_path_c(const struct file *f)
> >  {
> >  	return &backing_file(f)->user_path;
> >  }
> > -EXPORT_SYMBOL_GPL(backing_file_user_path);
> > +EXPORT_SYMBOL_GPL(backing_file_user_path_c);
> 
> May I suggest:
> 
> #define backing_file_user_path(f)	(_Generic((f),		\
> 	const struct file *:	(const struct path *)&backing_file(f)->user_path,  \
> 	struct file *:		(struct path *)&backing_file(f)->user_path)))

Um...  You do realize that backing_file_user_path() has users in places
that do *NOT* see the definition of struct backing_file?