| Message ID | 7e8c3a3c9a587b9cd8a2f146df32a421b961f3a2.1584432148.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/hugetlb: Fix build failure with HUGETLB_PAGE but not HUGEBTLBFS | expand |
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 <lkp@intel.com> > Link: https://patchwork.ozlabs.org/patch/1255548/#2386036 > Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes") > Cc: stable@vger.kernel.org > Signed-off-by: Christophe Leroy <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; > -- > 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 <lkp@intel.com> >> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036 >> Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes") >> Cc: stable@vger.kernel.org >> Signed-off-by: Christophe Leroy <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; >> -- >> 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.
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.
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 <lkp@intel.com> > Link: https://patchwork.ozlabs.org/patch/1255548/#2386036 > Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes") > Cc: stable@vger.kernel.org > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> 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 <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.
On Tue, 17 Mar 2020 08:04:14 +0000 (UTC) Christophe Leroy <christophe.leroy@c-s.fr> 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 <lkp@intel.com> > 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: stable@vger.kernel.org I think I'll remove this. Obviously nobody is suffering from this problem!
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;
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 <lkp@intel.com> Link: https://patchwork.ozlabs.org/patch/1255548/#2386036 Fixes: a137e1cc6d6e ("hugetlbfs: per mount huge page sizes") Cc: stable@vger.kernel.org Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> --- include/linux/hugetlb.h | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)