[PATCH] mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS

[Christophe Leroy]

When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
following build failure is encoutered:

In file included from arch/powerpc/mm/fault.c:33:0:
./include/linux/hugetlb.h: In function 'hstate_inode':
./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
return HUGETLBFS_SB(i->i_sb)->hstate;
^
./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
return HUGETLBFS_SB(i->i_sb)->hstate;
^

Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.

Reported-by: kbuild test robot <***@intel.com>
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
Cc: ***@vger.kernel.org
Signed-off-by: Christophe Leroy <***@c-s.fr>
---
include/linux/hugetlb.h | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 1e897e4168ac..dafb3d70ff81 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
return is_file_shm_hugepages(file);
}

-
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return HUGETLBFS_SB(i->i_sb)->hstate;
+}
#else /* !CONFIG_HUGETLBFS */

#define is_file_hugepages(file) false
@@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
return ERR_PTR(-ENOSYS);
}

+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return NULL;
+}
#endif /* !CONFIG_HUGETLBFS */

#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
@@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;

#define default_hstate (hstates[default_hstate_idx])

-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return HUGETLBFS_SB(i->i_sb)->hstate;
-}
-
static inline struct hstate *hstate_file(struct file *f)
{
return hstate_inode(file_inode(f));
@@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
return NULL;
}

-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return NULL;
-}
-
static inline struct hstate *page_hstate(struct page *page)
{
return NULL;

[Baoquan He]

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the

From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.

config HUGETLB_PAGE
def_bool HUGETLBFS

Post by Christophe Leroy
./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
return HUGETLBFS_SB(i->i_sb)->hstate;
^
./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
return HUGETLBFS_SB(i->i_sb)->hstate;
^
Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
---
include/linux/hugetlb.h | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 1e897e4168ac..dafb3d70ff81 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
return is_file_shm_hugepages(file);
}
-
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return HUGETLBFS_SB(i->i_sb)->hstate;
+}
#else /* !CONFIG_HUGETLBFS */
#define is_file_hugepages(file) false
@@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
return ERR_PTR(-ENOSYS);
}
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return NULL;
+}
#endif /* !CONFIG_HUGETLBFS */
#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
@@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
#define default_hstate (hstates[default_hstate_idx])
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return HUGETLBFS_SB(i->i_sb)->hstate;
-}
-
static inline struct hstate *hstate_file(struct file *f)
{
return hstate_inode(file_inode(f));
@@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
return NULL;
}
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return NULL;
-}
-
static inline struct hstate *page_hstate(struct page *page)
{
return NULL;
--
2.25.0

[Christophe Leroy]

Post by Baoquan He

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the

From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.

AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default
HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still
possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS
when it uses huge pages for other purpose than hugetlb file system.

Christophe

Post by Baoquan He
config HUGETLB_PAGE
def_bool HUGETLBFS

Post by Christophe Leroy
./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
return HUGETLBFS_SB(i->i_sb)->hstate;
^
./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
return HUGETLBFS_SB(i->i_sb)->hstate;
^
Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
---
include/linux/hugetlb.h | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 1e897e4168ac..dafb3d70ff81 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -390,7 +390,10 @@ static inline bool is_file_hugepages(struct file *file)
return is_file_shm_hugepages(file);
}
-
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return HUGETLBFS_SB(i->i_sb)->hstate;
+}
#else /* !CONFIG_HUGETLBFS */
#define is_file_hugepages(file) false
@@ -402,6 +405,10 @@ hugetlb_file_setup(const char *name, size_t size, vm_flags_t acctflag,
return ERR_PTR(-ENOSYS);
}
+static inline struct hstate *hstate_inode(struct inode *i)
+{
+ return NULL;
+}
#endif /* !CONFIG_HUGETLBFS */
#ifdef HAVE_ARCH_HUGETLB_UNMAPPED_AREA
@@ -472,11 +479,6 @@ extern unsigned int default_hstate_idx;
#define default_hstate (hstates[default_hstate_idx])
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return HUGETLBFS_SB(i->i_sb)->hstate;
-}
-
static inline struct hstate *hstate_file(struct file *f)
{
return hstate_inode(file_inode(f));
@@ -729,11 +731,6 @@ static inline struct hstate *hstate_vma(struct vm_area_struct *vma)
return NULL;
}
-static inline struct hstate *hstate_inode(struct inode *i)
-{
- return NULL;
-}
-
static inline struct hstate *page_hstate(struct page *page)
{
return NULL;
--
2.25.0

[Mike Kravetz]

Post by Baoquan He

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the

From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.

AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS when it uses huge pages for other purpose than hugetlb file system.

Hi Christophe,

Do you actually have a use case/example of using hugetlb pages without
hugetlbfs? I can understand that there are some use cases which never
use the filesystem interface. However, hugetlb support is so intertwined
with hugetlbfs, I am thinking there would be issues trying to use them
separately. I will look into this further.

[Christophe Leroy]

Post by Mike Kravetz

Post by Baoquan He

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the

From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.

AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS when it uses huge pages for other purpose than hugetlb file system.

Hi Christophe,
Do you actually have a use case/example of using hugetlb pages without
hugetlbfs? I can understand that there are some use cases which never
use the filesystem interface. However, hugetlb support is so intertwined
with hugetlbfs, I am thinking there would be issues trying to use them
separately. I will look into this further.

Hi Mike,

Series https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=164620

And especially patch 39 to 41.

Thanks
Christophe

[Mike Kravetz]

Post by Christophe Leroy

Post by Mike Kravetz

Post by Baoquan He

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the

From the definition of HUGETLB_PAGE, isn't it relying on HUGETLBFS?
I could misunderstand the def_bool, please correct me if I am wrong.

AFAIU, it means that HUGETLBFS rely on HUGETLB_PAGE, by default HUGETLB_PAGE is not selected when HUGETLBFS is not. But it is still possible for an arch to select HUGETLB_PAGE without selecting HUGETLBFS when it uses huge pages for other purpose than hugetlb file system.

Hi Christophe,
Do you actually have a use case/example of using hugetlb pages without
hugetlbfs? I can understand that there are some use cases which never
use the filesystem interface. However, hugetlb support is so intertwined
with hugetlbfs, I am thinking there would be issues trying to use them
separately. I will look into this further.

Hi Mike,
Series https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=164620
And especially patch 39 to 41.

Ah, ok. You are simply using a few interfaces in the hugetlb header files.
The huge pages created in your mappings are not PageHuge() pages.

[Mike Kravetz]

Post by Christophe Leroy
When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
./include/linux/hugetlb.h:477:9: error: implicit declaration of function 'HUGETLBFS_SB' [-Werror=implicit-function-declaration]
return HUGETLBFS_SB(i->i_sb)->hstate;
^
./include/linux/hugetlb.h:477:30: error: invalid type argument of '->' (have 'int')
return HUGETLBFS_SB(i->i_sb)->hstate;
^
Gate hstate_inode() with CONFIG_HUGETLBFS instead of CONFIG_HUGETLB_PAGE.
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")

As hugetlb.h evolved over time, I suspect nobody imagined a configuration
with CONFIG_HUGETLB_PAGE and not CONFIG_HUGETLBFS. This patch does address
the build issues. So,

Reviewed-by: Mike Kravetz <***@oracle.com>

However, there are many definitions in that file not behind #ifdef
CONFIG_HUGETLBFS that make no sense unless CONFIG_HUGETLBFS is defined.
Such cleanup is way beyond the scope of this patch/effort. I will add
it to the list of hugetlb/hugetlbfs things that can be cleaned up.