[PATCH] fs: Add FOP_HUGE_PAGES

[PATCH] fs: Add FOP_HUGE_PAGES

[Matthew Wilcox (Oracle)]

Instead of checking for specific file_operations, add a bit to
file_operations which denotes a file that only contain hugetlb pages.
This lets us make hugetlbfs_file_operations static, and removes
is_file_shm_hugepages() completely.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 fs/hugetlbfs/inode.c    |  5 +++--
 include/linux/fs.h      |  2 ++
 include/linux/hugetlb.h |  8 ++------
 include/linux/shm.h     |  5 -----
 ipc/shm.c               | 10 +++-------
 5 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 2f4e88552d3f..412f295acebe 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -40,7 +40,7 @@
 #include <linux/sched/mm.h>
 
 static const struct address_space_operations hugetlbfs_aops;
-const struct file_operations hugetlbfs_file_operations;
+static const struct file_operations hugetlbfs_file_operations;
 static const struct inode_operations hugetlbfs_dir_inode_operations;
 static const struct inode_operations hugetlbfs_inode_operations;
 
@@ -1298,13 +1298,14 @@ static void init_once(void *foo)
 	inode_init_once(&ei->vfs_inode);
 }
 
-const struct file_operations hugetlbfs_file_operations = {
+static const struct file_operations hugetlbfs_file_operations = {
 	.read_iter		= hugetlbfs_read_iter,
 	.mmap			= hugetlbfs_file_mmap,
 	.fsync			= noop_fsync,
 	.get_unmapped_area	= hugetlb_get_unmapped_area,
 	.llseek			= default_llseek,
 	.fallocate		= hugetlbfs_fallocate,
+	.fop_flags		= FOP_HUGE_PAGES,
 };
 
 static const struct inode_operations hugetlbfs_dir_inode_operations = {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 883b72478f61..736ffe2c6863 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2043,6 +2043,8 @@ struct file_operations {
 #define FOP_MMAP_SYNC		((__force fop_flags_t)(1 << 2))
 /* Supports non-exclusive O_DIRECT writes from multiple threads */
 #define FOP_DIO_PARALLEL_WRITE	((__force fop_flags_t)(1 << 3))
+/* Contains huge pages */
+#define FOP_HUGE_PAGES		((__force fop_flags_t)(1 << 4))
 
 /* Wrap a directory iterator that needs exclusive inode access */
 int wrap_directory_iterator(struct file *, struct dir_context *,
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index bebf4c3a53ef..aaa11eeb939d 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -526,17 +526,13 @@ static inline struct hugetlbfs_inode_info *HUGETLBFS_I(struct inode *inode)
 	return container_of(inode, struct hugetlbfs_inode_info, vfs_inode);
 }
 
-extern const struct file_operations hugetlbfs_file_operations;
 extern const struct vm_operations_struct hugetlb_vm_ops;
 struct file *hugetlb_file_setup(const char *name, size_t size, vm_flags_t acct,
 				int creat_flags, int page_size_log);
 
-static inline bool is_file_hugepages(struct file *file)
+static inline bool is_file_hugepages(const struct file *file)
 {
-	if (file->f_op == &hugetlbfs_file_operations)
-		return true;
-
-	return is_file_shm_hugepages(file);
+	return file->f_op->fop_flags & FOP_HUGE_PAGES;
 }
 
 static inline struct hstate *hstate_inode(struct inode *i)
diff --git a/include/linux/shm.h b/include/linux/shm.h
index c55bef0538e5..1d3d3ae958fb 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -16,7 +16,6 @@ struct sysv_shm {
 
 long do_shmat(int shmid, char __user *shmaddr, int shmflg, unsigned long *addr,
 	      unsigned long shmlba);
-bool is_file_shm_hugepages(struct file *file);
 void exit_shm(struct task_struct *task);
 #define shm_init_task(task) INIT_LIST_HEAD(&(task)->sysvshm.shm_clist)
 #else
@@ -30,10 +29,6 @@ static inline long do_shmat(int shmid, char __user *shmaddr,
 {
 	return -ENOSYS;
 }
-static inline bool is_file_shm_hugepages(struct file *file)
-{
-	return false;
-}
 static inline void exit_shm(struct task_struct *task)
 {
 }
diff --git a/ipc/shm.c b/ipc/shm.c
index a89f001a8bf0..3e3071252dac 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -662,8 +662,8 @@ static const struct file_operations shm_file_operations = {
 };
 
 /*
- * shm_file_operations_huge is now identical to shm_file_operations,
- * but we keep it distinct for the sake of is_file_shm_hugepages().
+ * shm_file_operations_huge is now identical to shm_file_operations
+ * except for fop_flags
  */
 static const struct file_operations shm_file_operations_huge = {
 	.mmap		= shm_mmap,
@@ -672,13 +672,9 @@ static const struct file_operations shm_file_operations_huge = {
 	.get_unmapped_area	= shm_get_unmapped_area,
 	.llseek		= noop_llseek,
 	.fallocate	= shm_fallocate,
+	.fop_flags	= FOP_HUGE_PAGES,
 };
 
-bool is_file_shm_hugepages(struct file *file)
-{
-	return file->f_op == &shm_file_operations_huge;
-}
-
 static const struct vm_operations_struct shm_vm_ops = {
 	.open	= shm_open,	/* callback for a new vm-area open */
 	.close	= shm_close,	/* callback for when the vm-area is released */
-- 
2.43.0

Re: [PATCH] fs: Add FOP_HUGE_PAGES

[Al Viro]

On Sun, Apr 07, 2024 at 09:11:20PM +0100, Matthew Wilcox (Oracle) wrote:
> -static inline bool is_file_hugepages(struct file *file)
> +static inline bool is_file_hugepages(const struct file *file)
>  {
> -	if (file->f_op == &hugetlbfs_file_operations)
> -		return true;
> -
> -	return is_file_shm_hugepages(file);
> +	return file->f_op->fop_flags & FOP_HUGE_PAGES;
>  }

Extra cacheline to pull can be costly on a sufficiently hot path...

Re: [PATCH] fs: Add FOP_HUGE_PAGES

[Matthew Wilcox]

On Mon, Apr 08, 2024 at 04:02:17PM +0100, Al Viro wrote:
> On Sun, Apr 07, 2024 at 09:11:20PM +0100, Matthew Wilcox (Oracle) wrote:
> > -static inline bool is_file_hugepages(struct file *file)
> > +static inline bool is_file_hugepages(const struct file *file)
> >  {
> > -	if (file->f_op == &hugetlbfs_file_operations)
> > -		return true;
> > -
> > -	return is_file_shm_hugepages(file);
> > +	return file->f_op->fop_flags & FOP_HUGE_PAGES;
> >  }
> 
> Extra cacheline to pull can be costly on a sufficiently hot path...

Sure, but so can a function call, particularly with the $%^&@#
CPU mitigations.  Yes, is_file_hugepages() is inline, but
is_file_shm_hugepages() is not.  The cacheline in question should be
shared since it's part of fops, which is const.

I'm not seeing is_file_hugepages() called on any partcularly hot paths.
Most of the callers are in mmap() equivalent paths.

Re: [PATCH] fs: Add FOP_HUGE_PAGES

[Christian Brauner]

On Sun, 07 Apr 2024 21:11:20 +0100, Matthew Wilcox (Oracle) wrote:
> Instead of checking for specific file_operations, add a bit to
> file_operations which denotes a file that only contain hugetlb pages.
> This lets us make hugetlbfs_file_operations static, and removes
> is_file_shm_hugepages() completely.
> 
> 

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/1] fs: Add FOP_HUGE_PAGES
      https://git.kernel.org/vfs/vfs/c/886b94d25a8e