linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/hmm: fix unused variable warnings
@ 2019-03-04 20:00 Arnd Bergmann
  2019-03-05 12:18 ` Anshuman Khandual
  2019-03-05 23:51 ` John Hubbard
  0 siblings, 2 replies; 6+ messages in thread
From: Arnd Bergmann @ 2019-03-04 20:00 UTC (permalink / raw)
  To: Jérôme Glisse
  Cc: Arnd Bergmann, Andrew Morton, Ralph Campbell, Stephen Rothwell,
	John Hubbard, Dan Williams, linux-mm, linux-kernel

When CONFIG_HUGETLB_PAGE is disabled, the only use of the variable 'h'
is compiled out, and the compiler thinks it is unnecessary:

mm/hmm.c: In function 'hmm_range_snapshot':
mm/hmm.c:1015:19: error: unused variable 'h' [-Werror=unused-variable]
    struct hstate *h = hstate_vma(vma);

Rephrase the code to avoid the temporary variable instead, so the
compiler stops warning.

Fixes: 5409a90d4212 ("mm/hmm: support hugetlbfs (snapshotting, faulting and DMA mapping)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 mm/hmm.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/mm/hmm.c b/mm/hmm.c
index 3c9781037918..c4beb1628cad 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -1012,9 +1012,8 @@ long hmm_range_snapshot(struct hmm_range *range)
 			return -EFAULT;
 
 		if (is_vm_hugetlb_page(vma)) {
-			struct hstate *h = hstate_vma(vma);
-
-			if (huge_page_shift(h) != range->page_shift &&
+			if (range->page_shift !=
+				huge_page_shift(hstate_vma(vma)) &&
 			    range->page_shift != PAGE_SHIFT)
 				return -EINVAL;
 		} else {
@@ -1115,9 +1114,8 @@ long hmm_range_fault(struct hmm_range *range, bool block)
 			return -EFAULT;
 
 		if (is_vm_hugetlb_page(vma)) {
-			struct hstate *h = hstate_vma(vma);
-
-			if (huge_page_shift(h) != range->page_shift &&
+			if (range->page_shift !=
+				huge_page_shift(hstate_vma(vma)) &&
 			    range->page_shift != PAGE_SHIFT)
 				return -EINVAL;
 		} else {
-- 
2.20.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/hmm: fix unused variable warnings
  2019-03-04 20:00 [PATCH] mm/hmm: fix unused variable warnings Arnd Bergmann
@ 2019-03-05 12:18 ` Anshuman Khandual
  2019-03-06 10:26   ` Arnd Bergmann
  2019-03-05 23:51 ` John Hubbard
  1 sibling, 1 reply; 6+ messages in thread
From: Anshuman Khandual @ 2019-03-05 12:18 UTC (permalink / raw)
  To: Arnd Bergmann, Jérôme Glisse
  Cc: Andrew Morton, Ralph Campbell, Stephen Rothwell, John Hubbard,
	Dan Williams, linux-mm, linux-kernel



On 03/05/2019 01:30 AM, Arnd Bergmann wrote:
> When CONFIG_HUGETLB_PAGE is disabled, the only use of the variable 'h'
> is compiled out, and the compiler thinks it is unnecessary:
> 
> mm/hmm.c: In function 'hmm_range_snapshot':
> mm/hmm.c:1015:19: error: unused variable 'h' [-Werror=unused-variable]
>     struct hstate *h = hstate_vma(vma);

After doing some Kconfig hacks like (ARCH_WANT_GENERAL_HUGETLB = n) on an
X86 system I got (HUGETLB_PAGE = n and HMM = y) config. But was unable to
hit the build error. Helper is_vm_hugetlb_page() seems to always return
false when HUGETLB_PAGE = n. Would not the compiler remove the entire code
block including the declaration for 'h' ?

#ifdef CONFIG_HUGETLB_PAGE
#include <linux/mm.h>
static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
{
        return !!(vma->vm_flags & VM_HUGETLB);
}
#else
static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
{
        return false;
}
#endif

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/hmm: fix unused variable warnings
  2019-03-04 20:00 [PATCH] mm/hmm: fix unused variable warnings Arnd Bergmann
  2019-03-05 12:18 ` Anshuman Khandual
@ 2019-03-05 23:51 ` John Hubbard
  2019-03-06 10:19   ` Arnd Bergmann
  1 sibling, 1 reply; 6+ messages in thread
From: John Hubbard @ 2019-03-05 23:51 UTC (permalink / raw)
  To: Arnd Bergmann, Jérôme Glisse
  Cc: Andrew Morton, Ralph Campbell, Stephen Rothwell, Dan Williams,
	linux-mm, linux-kernel

On 3/4/19 12:00 PM, Arnd Bergmann wrote:
> When CONFIG_HUGETLB_PAGE is disabled, the only use of the variable 'h'
> is compiled out, and the compiler thinks it is unnecessary:
> 
> mm/hmm.c: In function 'hmm_range_snapshot':
> mm/hmm.c:1015:19: error: unused variable 'h' [-Werror=unused-variable]
>     struct hstate *h = hstate_vma(vma);
> 
> Rephrase the code to avoid the temporary variable instead, so the
> compiler stops warning.
> 
> Fixes: 5409a90d4212 ("mm/hmm: support hugetlbfs (snapshotting, faulting and DMA mapping)")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  mm/hmm.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 3c9781037918..c4beb1628cad 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -1012,9 +1012,8 @@ long hmm_range_snapshot(struct hmm_range *range)
>  			return -EFAULT;
>  
>  		if (is_vm_hugetlb_page(vma)) {
> -			struct hstate *h = hstate_vma(vma);
> -
> -			if (huge_page_shift(h) != range->page_shift &&
> +			if (range->page_shift !=
> +				huge_page_shift(hstate_vma(vma)) &&
>  			    range->page_shift != PAGE_SHIFT)
>  				return -EINVAL;
>  		} else {
> @@ -1115,9 +1114,8 @@ long hmm_range_fault(struct hmm_range *range, bool block)
>  			return -EFAULT;
>  
>  		if (is_vm_hugetlb_page(vma)) {
> -			struct hstate *h = hstate_vma(vma);
> -
> -			if (huge_page_shift(h) != range->page_shift &&
> +			if (range->page_shift !=
> +				huge_page_shift(hstate_vma(vma)) &&
>  			    range->page_shift != PAGE_SHIFT)
>  				return -EINVAL;
>  		} else {
> 

Hi Arnd,

With some Kconfig local hacks that removed all HUGE* support, while leaving
HMM enabled, I was able to reproduce your results, and also to verify the
fix. It also makes sense from reading it.

Also, I ran into one more warning as well:

mm/hmm.c: In function ‘hmm_vma_walk_pud’:
mm/hmm.c:764:25: warning: unused variable ‘vma’ [-Wunused-variable]
  struct vm_area_struct *vma = walk->vma;
                         ^~~

...which can be fixed like this:

diff --git a/mm/hmm.c b/mm/hmm.c
index c4beb1628cad..c1cbe82d12b5 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -761,7 +761,6 @@ static int hmm_vma_walk_pud(pud_t *pudp,
 {
        struct hmm_vma_walk *hmm_vma_walk = walk->private;
        struct hmm_range *range = hmm_vma_walk->range;
-       struct vm_area_struct *vma = walk->vma;
        unsigned long addr = start, next;
        pmd_t *pmdp;
        pud_t pud;
@@ -807,7 +806,7 @@ static int hmm_vma_walk_pud(pud_t *pudp,
                return 0;
        }
 
-       split_huge_pud(vma, pudp, addr);
+       split_huge_pud(walk->vma, pudp, addr);
        if (pud_none(*pudp))
                goto again;

...so maybe you'd like to fold that into your patch?



thanks,
-- 
John Hubbard
NVIDIA

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/hmm: fix unused variable warnings
  2019-03-05 23:51 ` John Hubbard
@ 2019-03-06 10:19   ` Arnd Bergmann
  2019-03-06 18:34     ` John Hubbard
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2019-03-06 10:19 UTC (permalink / raw)
  To: John Hubbard
  Cc: Jérôme Glisse, Andrew Morton, Ralph Campbell,
	Stephen Rothwell, Dan Williams, Linux-MM,
	Linux Kernel Mailing List

On Wed, Mar 6, 2019 at 12:51 AM John Hubbard <jhubbard@nvidia.com> wrote:
>
> With some Kconfig local hacks that removed all HUGE* support, while leaving
> HMM enabled, I was able to reproduce your results, and also to verify the
> fix. It also makes sense from reading it.

Thanks for the confirmation.

> Also, I ran into one more warning as well:
>
> mm/hmm.c: In function ‘hmm_vma_walk_pud’:
> mm/hmm.c:764:25: warning: unused variable ‘vma’ [-Wunused-variable]
>   struct vm_area_struct *vma = walk->vma;
>                          ^~~
>
> ...which can be fixed like this:
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index c4beb1628cad..c1cbe82d12b5 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -761,7 +761,6 @@ static int hmm_vma_walk_pud(pud_t *pudp,
>  {
>         struct hmm_vma_walk *hmm_vma_walk = walk->private;
>         struct hmm_range *range = hmm_vma_walk->range;
> -       struct vm_area_struct *vma = walk->vma;
>         unsigned long addr = start, next;
>         pmd_t *pmdp;
>         pud_t pud;
> @@ -807,7 +806,7 @@ static int hmm_vma_walk_pud(pud_t *pudp,
>                 return 0;
>         }
>
> -       split_huge_pud(vma, pudp, addr);
> +       split_huge_pud(walk->vma, pudp, addr);
>         if (pud_none(*pudp))
>                 goto again;
>
> ...so maybe you'd like to fold that into your patch?

I also ran into this one last night during further randconfig testing,
and came up with the same patch that you showed here. I'll
send this one to Andrew and add a Reported-by line for you,
since he already merged the first patch.

I'll leave it up to Andrew to fold the fixes into one, or into the original
patches if he thinks that makes sense.

     Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/hmm: fix unused variable warnings
  2019-03-05 12:18 ` Anshuman Khandual
@ 2019-03-06 10:26   ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2019-03-06 10:26 UTC (permalink / raw)
  To: Anshuman Khandual
  Cc: Jérôme Glisse, Andrew Morton, Ralph Campbell,
	Stephen Rothwell, John Hubbard, Dan Williams, Linux-MM,
	Linux Kernel Mailing List

On Tue, Mar 5, 2019 at 1:18 PM Anshuman Khandual
<anshuman.khandual@arm.com> wrote:
> On 03/05/2019 01:30 AM, Arnd Bergmann wrote:
> > When CONFIG_HUGETLB_PAGE is disabled, the only use of the variable 'h'
> > is compiled out, and the compiler thinks it is unnecessary:
> >
> > mm/hmm.c: In function 'hmm_range_snapshot':
> > mm/hmm.c:1015:19: error: unused variable 'h' [-Werror=unused-variable]
> >     struct hstate *h = hstate_vma(vma);
>
> After doing some Kconfig hacks like (ARCH_WANT_GENERAL_HUGETLB = n) on an
> X86 system I got (HUGETLB_PAGE = n and HMM = y) config. But was unable to
> hit the build error. Helper is_vm_hugetlb_page() seems to always return
> false when HUGETLB_PAGE = n. Would not the compiler remove the entire code
> block including the declaration for 'h' ?
>
> #ifdef CONFIG_HUGETLB_PAGE
> #include <linux/mm.h>
> static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
> {
>         return !!(vma->vm_flags & VM_HUGETLB);
> }
> #else
> static inline bool is_vm_hugetlb_page(struct vm_area_struct *vma)
> {
>         return false;
> }
> #endif

The is_vm_hugetlb_page() check is unrelated to the warning here,
the problem is that huge_page_shift() is defined as

#define huge_page_shift(h) PAGE_SHIFT

when CONFIG_HUGETLB_PAGE is disabled, so after preprocessing,
the only reference to the variable is removed.

     Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mm/hmm: fix unused variable warnings
  2019-03-06 10:19   ` Arnd Bergmann
@ 2019-03-06 18:34     ` John Hubbard
  0 siblings, 0 replies; 6+ messages in thread
From: John Hubbard @ 2019-03-06 18:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jérôme Glisse, Andrew Morton, Ralph Campbell,
	Stephen Rothwell, Dan Williams, Linux-MM,
	Linux Kernel Mailing List

On 3/6/19 2:19 AM, Arnd Bergmann wrote:
> On Wed, Mar 6, 2019 at 12:51 AM John Hubbard <jhubbard@nvidia.com> wrote:
>>
>> With some Kconfig local hacks that removed all HUGE* support, while leaving
>> HMM enabled, I was able to reproduce your results, and also to verify the
>> fix. It also makes sense from reading it.
> 
> Thanks for the confirmation.
> 
>> Also, I ran into one more warning as well:
>>
>> mm/hmm.c: In function ‘hmm_vma_walk_pud’:
>> mm/hmm.c:764:25: warning: unused variable ‘vma’ [-Wunused-variable]
>>   struct vm_area_struct *vma = walk->vma;
>>                          ^~~
>>
>> ...which can be fixed like this:
>>
>> diff --git a/mm/hmm.c b/mm/hmm.c
>> index c4beb1628cad..c1cbe82d12b5 100644
>> --- a/mm/hmm.c
>> +++ b/mm/hmm.c
>> @@ -761,7 +761,6 @@ static int hmm_vma_walk_pud(pud_t *pudp,
>>  {
>>         struct hmm_vma_walk *hmm_vma_walk = walk->private;
>>         struct hmm_range *range = hmm_vma_walk->range;
>> -       struct vm_area_struct *vma = walk->vma;
>>         unsigned long addr = start, next;
>>         pmd_t *pmdp;
>>         pud_t pud;
>> @@ -807,7 +806,7 @@ static int hmm_vma_walk_pud(pud_t *pudp,
>>                 return 0;
>>         }
>>
>> -       split_huge_pud(vma, pudp, addr);
>> +       split_huge_pud(walk->vma, pudp, addr);
>>         if (pud_none(*pudp))
>>                 goto again;
>>
>> ...so maybe you'd like to fold that into your patch?
> 
> I also ran into this one last night during further randconfig testing,
> and came up with the same patch that you showed here. I'll
> send this one to Andrew and add a Reported-by line for you,
> since he already merged the first patch.
> 
> I'll leave it up to Andrew to fold the fixes into one, or into the original
> patches if he thinks that makes sense.
> 
>      Arnd
> 

Sounds good!

thanks,
-- 
John Hubbard
NVIDIA

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-03-06 18:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 20:00 [PATCH] mm/hmm: fix unused variable warnings Arnd Bergmann
2019-03-05 12:18 ` Anshuman Khandual
2019-03-06 10:26   ` Arnd Bergmann
2019-03-05 23:51 ` John Hubbard
2019-03-06 10:19   ` Arnd Bergmann
2019-03-06 18:34     ` John Hubbard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).