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 <[email protected]>
Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
Cc: [email protected]
Signed-off-by: Christophe Leroy <[email protected]>
---
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
On 03/17/20 at 08:04am, Christophe Leroy wrote:
> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
> following build failure is encoutered:
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
>
> 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 <[email protected]>
> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
> Cc: [email protected]
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> 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
>
>
Le 17/03/2020 à 09:25, Baoquan He a écrit :
> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>> following build failure is encoutered:
>
> 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
>
> config HUGETLB_PAGE
> def_bool HUGETLBFS
>
>>
>> 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 <[email protected]>
>> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
>> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
>> Cc: [email protected]
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> 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
>>
>>
On 3/17/20 1:43 AM, Christophe Leroy wrote:
>
>
> Le 17/03/2020 à 09:25, Baoquan He a écrit :
>> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>>> following build failure is encoutered:
>>
>> 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.
--
Mike Kravetz
Le 17/03/2020 à 17:40, Mike Kravetz a écrit :
> On 3/17/20 1:43 AM, Christophe Leroy wrote:
>>
>>
>> Le 17/03/2020 à 09:25, Baoquan He a écrit :
>>> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>>>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>>>> following build failure is encoutered:
>>>
>>> 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
On 3/17/20 9:47 AM, Christophe Leroy wrote:
>
>
> Le 17/03/2020 à 17:40, Mike Kravetz a écrit :
>> On 3/17/20 1:43 AM, Christophe Leroy wrote:
>>>
>>>
>>> Le 17/03/2020 à 09:25, Baoquan He a écrit :
>>>> On 03/17/20 at 08:04am, Christophe Leroy wrote:
>>>>> When CONFIG_HUGETLB_PAGE is set but not CONFIG_HUGETLBFS, the
>>>>> following build failure is encoutered:
>>>>
>>>> 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
On 3/17/20 1:04 AM, Christophe Leroy wrote:
> 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 <[email protected]>
> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
> Cc: [email protected]
> Signed-off-by: Christophe Leroy <[email protected]>
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 <[email protected]>
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.
--
Mike Kravetz
On Tue, 17 Mar 2020 08:04:14 +0000 (UTC) Christophe Leroy <[email protected]> wrote:
> 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 <[email protected]>
> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036
> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes")
A 12 year old build error? If accurate, that has to be a world record.
> Cc: [email protected]
I think I'll remove this. Obviously nobody is suffering from this problem!