[PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[Alexandre Ghiti]

The only definition of vmf_insert_pfn_pud depends on
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
include/linux/huge_mm.h should have the same restriction so that we do
not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
not defined.

Signed-off-by: Alexandre Ghiti <aghiti@upmem.com>
---
 include/linux/huge_mm.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index a8a1262..11794f6a 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -48,8 +48,10 @@ extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
 			int prot_numa);
 int vmf_insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
 			pmd_t *pmd, pfn_t pfn, bool write);
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 int vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
 			pud_t *pud, pfn_t pfn, bool write);
+#endif
 enum transparent_hugepage_flag {
 	TRANSPARENT_HUGEPAGE_FLAG,
 	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[Michal Hocko]

On Thu 11-01-18 09:53:31, Alexandre Ghiti wrote:
> The only definition of vmf_insert_pfn_pud depends on
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
> include/linux/huge_mm.h should have the same restriction so that we do
> not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
> not defined.

Why is this a problem? Compiler should simply throw away any
declarations which are not used?

> Signed-off-by: Alexandre Ghiti <aghiti@upmem.com>
> ---
>  include/linux/huge_mm.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index a8a1262..11794f6a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -48,8 +48,10 @@ extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>  			int prot_numa);
>  int vmf_insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
>  			pmd_t *pmd, pfn_t pfn, bool write);
> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>  int vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
>  			pud_t *pud, pfn_t pfn, bool write);
> +#endif
>  enum transparent_hugepage_flag {
>  	TRANSPARENT_HUGEPAGE_FLAG,
>  	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
> -- 
> 2.1.4
> 

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[Alexandre Ghiti]

On 11/01/2018 11:06, Michal Hocko wrote:
> On Thu 11-01-18 09:53:31, Alexandre Ghiti wrote:
>> The only definition of vmf_insert_pfn_pud depends on
>> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
>> include/linux/huge_mm.h should have the same restriction so that we do
>> not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
>> not defined.
> Why is this a problem? Compiler should simply throw away any
> declarations which are not used?
It is not a big problem but surrounding the declaration with the #ifdef 
makes the compilation of external modules fail with an "error: implicit 
declaration of function vmf_insert_pfn_pud" if 
CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is not defined. I think it is 
cleaner than generating a .ko which would not load anyway.

>
>> Signed-off-by: Alexandre Ghiti <aghiti@upmem.com>
>> ---
>>   include/linux/huge_mm.h | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index a8a1262..11794f6a 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -48,8 +48,10 @@ extern int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
>>   			int prot_numa);
>>   int vmf_insert_pfn_pmd(struct vm_area_struct *vma, unsigned long addr,
>>   			pmd_t *pmd, pfn_t pfn, bool write);
>> +#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
>>   int vmf_insert_pfn_pud(struct vm_area_struct *vma, unsigned long addr,
>>   			pud_t *pud, pfn_t pfn, bool write);
>> +#endif
>>   enum transparent_hugepage_flag {
>>   	TRANSPARENT_HUGEPAGE_FLAG,
>>   	TRANSPARENT_HUGEPAGE_REQ_MADV_FLAG,
>> -- 
>> 2.1.4
>>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[Andrew Morton]

On Thu, 11 Jan 2018 14:05:34 +0100 Alexandre Ghiti <aghiti@upmem.com> wrote:

> On 11/01/2018 11:06, Michal Hocko wrote:
> > On Thu 11-01-18 09:53:31, Alexandre Ghiti wrote:
> >> The only definition of vmf_insert_pfn_pud depends on
> >> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
> >> include/linux/huge_mm.h should have the same restriction so that we do
> >> not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
> >> not defined.
> > Why is this a problem? Compiler should simply throw away any
> > declarations which are not used?
> It is not a big problem but surrounding the declaration with the #ifdef 
> makes the compilation of external modules fail with an "error: implicit 
> declaration of function vmf_insert_pfn_pud" if 
> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is not defined. I think it is 
> cleaner than generating a .ko which would not load anyway.

Disagree.  We'd have to put an absolutely vast amount of complex and
hard-to-maintain ifdefs in headers if we were to ensure that such
errors were to be detected at compile time.

Whereas if we defer the detection of the errors until link time (or
depmod or modprobe time) then yes, a handful of people will detect
their mistake a minute or three later but that's a small cost compared
to permanently and badly messing up the header files.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm, THP: vmf_insert_pfn_pud depends on CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD

[Alexandre Ghiti]

On 12/01/2018 01:28, Andrew Morton wrote:
> On Thu, 11 Jan 2018 14:05:34 +0100 Alexandre Ghiti <aghiti@upmem.com> wrote:
>
>> On 11/01/2018 11:06, Michal Hocko wrote:
>>> On Thu 11-01-18 09:53:31, Alexandre Ghiti wrote:
>>>> The only definition of vmf_insert_pfn_pud depends on
>>>> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD being defined. Then its declaration in
>>>> include/linux/huge_mm.h should have the same restriction so that we do
>>>> not expose this function if CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is
>>>> not defined.
>>> Why is this a problem? Compiler should simply throw away any
>>> declarations which are not used?
>> It is not a big problem but surrounding the declaration with the #ifdef
>> makes the compilation of external modules fail with an "error: implicit
>> declaration of function vmf_insert_pfn_pud" if
>> CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD is not defined. I think it is
>> cleaner than generating a .ko which would not load anyway.
> Disagree.  We'd have to put an absolutely vast amount of complex and
> hard-to-maintain ifdefs in headers if we were to ensure that such
> errors were to be detected at compile time.
>
> Whereas if we defer the detection of the errors until link time (or
> depmod or modprobe time) then yes, a handful of people will detect
> their mistake a minute or three later but that's a small cost compared
> to permanently and badly messing up the header files.
Ok, thanks for your time and explanations.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>