Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1538308 > unrolled thread

[PATCH] x86/vm86: fix compilation warning on a unused variable

Started byJérémy Lefaure <jeremy.lefaure@lse.epita.fr>
First post2016-12-08 05:50 +0100
Last post2016-12-08 19:30 +0100
Articles 6 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86/vm86: fix compilation warning on a unused variable Jérémy Lefaure          <jeremy.lefaure@lse.epita.fr> - 2016-12-08 05:50 +0100
    Re: [PATCH] x86/vm86: fix compilation warning on a unused variable Borislav Petkov <bp@suse.de> - 2016-12-08 09:40 +0100
      Re: [PATCH] x86/vm86: fix compilation warning on a unused variable Jérémy Lefaure <jeremy.lefaure@lse.epita.fr> - 2016-12-08 19:00 +0100
    Re: [PATCH] x86/vm86: fix compilation warning on a unused variable "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> - 2016-12-08 12:00 +0100
      Re: [PATCH] x86/vm86: fix compilation warning on a unused variable "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-12-08 12:50 +0100
      Re: [PATCH] x86/vm86: fix compilation warning on a unused variable Jérémy Lefaure <jeremy.lefaure@lse.epita.fr> - 2016-12-08 19:30 +0100

#1538308 — [PATCH] x86/vm86: fix compilation warning on a unused variable

FromJérémy Lefaure <jeremy.lefaure@lse.epita.fr>
Date2016-12-08 05:50 +0100
Subject[PATCH] x86/vm86: fix compilation warning on a unused variable
Message-ID<sLYOB-1pF-1@gated-at.bofh.it>
When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
stub. In such case, vma is unused and a compiler raises a warning:

arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
[-Wunused-variable]
   struct vm_area_struct *vma = find_vma(mm, 0xA0000);
                             ^~~
Adding __maybe_unused in the vma declaration fixes this warning.

In addition, checking if CONFIG_TRANSPARENT_HUGEPAGE is enabled avoids
calling find_vma function for nothing.

Signed-off-by: Jérémy Lefaure <jeremy.lefaure@lse.epita.fr>
---
 arch/x86/kernel/vm86_32.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index 01f30e5..0813b76 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -176,8 +176,9 @@ static void mark_screen_rdonly(struct mm_struct *mm)
 		goto out;
 	pmd = pmd_offset(pud, 0xA0000);
 
-	if (pmd_trans_huge(*pmd)) {
-		struct vm_area_struct *vma = find_vma(mm, 0xA0000);
+	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pmd_trans_huge(*pmd)) {
+		struct vm_area_struct __maybe_unused *vma = find_vma(mm,
+								     0xA0000);
 		split_huge_pmd(vma, pmd, 0xA0000);
 	}
 	if (pmd_none_or_clear_bad(pmd))
-- 
2.10.2

[toc] | [next] | [standalone]

#1538378

FromBorislav Petkov <bp@suse.de>
Date2016-12-08 09:40 +0100
Message-ID<sM2pb-3S8-11@gated-at.bofh.it>
In reply to#1538308
On Wed, Dec 07, 2016 at 11:38:33PM -0500, Jérémy Lefaure wrote:
> When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
> stub. In such case, vma is unused and a compiler raises a warning:
> 
> arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
> arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
> [-Wunused-variable]
>    struct vm_area_struct *vma = find_vma(mm, 0xA0000);
>                              ^~~
> Adding __maybe_unused in the vma declaration fixes this warning.
> 
> In addition, checking if CONFIG_TRANSPARENT_HUGEPAGE is enabled avoids
> calling find_vma function for nothing.
> 
> Signed-off-by: Jérémy Lefaure <jeremy.lefaure@lse.epita.fr>
> ---
>  arch/x86/kernel/vm86_32.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
> index 01f30e5..0813b76 100644
> --- a/arch/x86/kernel/vm86_32.c
> +++ b/arch/x86/kernel/vm86_32.c
> @@ -176,8 +176,9 @@ static void mark_screen_rdonly(struct mm_struct *mm)
>  		goto out;
>  	pmd = pmd_offset(pud, 0xA0000);
>  
> -	if (pmd_trans_huge(*pmd)) {
> -		struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pmd_trans_huge(*pmd)) {
> +		struct vm_area_struct __maybe_unused *vma = find_vma(mm,
> +								     0xA0000);

So wouldn't the __maybe_unused alone without changing the if-condition
fix the warning too?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--

[toc] | [prev] | [next] | [standalone]

#1538767

FromJérémy Lefaure <jeremy.lefaure@lse.epita.fr>
Date2016-12-08 19:00 +0100
Message-ID<sMb97-Ct-17@gated-at.bofh.it>
In reply to#1538378
On Thu, 8 Dec 2016 09:33:05 +0100
Borislav Petkov <bp@suse.de> wrote:

> On Wed, Dec 07, 2016 at 11:38:33PM -0500, Jérémy Lefaure wrote:
> > When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
> > stub. In such case, vma is unused and a compiler raises a warning:
> > 
> > arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
> > arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
> > [-Wunused-variable]
> >    struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> >                              ^~~
> > Adding __maybe_unused in the vma declaration fixes this warning.
> > 
> > In addition, checking if CONFIG_TRANSPARENT_HUGEPAGE is enabled avoids
> > calling find_vma function for nothing.
> > 
> > Signed-off-by: Jérémy Lefaure <jeremy.lefaure@lse.epita.fr>
> > ---
> >  arch/x86/kernel/vm86_32.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
> > index 01f30e5..0813b76 100644
> > --- a/arch/x86/kernel/vm86_32.c
> > +++ b/arch/x86/kernel/vm86_32.c
> > @@ -176,8 +176,9 @@ static void mark_screen_rdonly(struct mm_struct *mm)
> >  		goto out;
> >  	pmd = pmd_offset(pud, 0xA0000);
> >  
> > -	if (pmd_trans_huge(*pmd)) {
> > -		struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> > +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE) && pmd_trans_huge(*pmd)) {
> > +		struct vm_area_struct __maybe_unused *vma = find_vma(mm,
> > +								     0xA0000);  
> 
> So wouldn't the __maybe_unused alone without changing the if-condition
> fix the warning too?
> 

Yes it will. I did not see that pmd_trans_huge returns 0 if
CONFIG_TRANSPARENT_HUGEPAGE is disabled. So you're right, the
IS_ENABLED(...) in the condition is useless.

Thanks,
Jérémy

[toc] | [prev] | [next] | [standalone]

#1538429

From"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date2016-12-08 12:00 +0100
Message-ID<sM4AF-57U-9@gated-at.bofh.it>
In reply to#1538308
On Wed, Dec 07, 2016 at 11:38:33PM -0500, Jérémy Lefaure wrote:
> When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
> stub. In such case, vma is unused and a compiler raises a warning:
> 
> arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
> arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
> [-Wunused-variable]
>    struct vm_area_struct *vma = find_vma(mm, 0xA0000);
>                              ^~~
> Adding __maybe_unused in the vma declaration fixes this warning.

Hm. pmd_trans_huge() is zero if CONFIG_TRANSPARENT_HUGEPAGE is not set.
Compiler should get rid of whole block of code under the 'if'.

Could you share your kernel config which triggers the warning?
And what compiler do you use?

-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1538462

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2016-12-08 12:50 +0100
Message-ID<sM5n3-5Di-11@gated-at.bofh.it>
In reply to#1538429
On Thu, Dec 08, 2016 at 01:50:11PM +0300, Kirill A. Shutemov wrote:
> On Wed, Dec 07, 2016 at 11:38:33PM -0500, Jérémy Lefaure wrote:
> > When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
> > stub. In such case, vma is unused and a compiler raises a warning:
> > 
> > arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
> > arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
> > [-Wunused-variable]
> >    struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> >                              ^~~
> > Adding __maybe_unused in the vma declaration fixes this warning.
> 
> Hm. pmd_trans_huge() is zero if CONFIG_TRANSPARENT_HUGEPAGE is not set.
> Compiler should get rid of whole block of code under the 'if'.
> 
> Could you share your kernel config which triggers the warning?
> And what compiler do you use?

Okay, I see the problem. It still doesn't make sense. Why would compiler
check for unused warnings before dropping unused code.

What about something like this, instead:

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 6f14de45b5ce..b538452a127e 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -180,7 +180,7 @@ static inline int split_huge_page(struct page *page)
 }
 static inline void deferred_split_huge_page(struct page *page) {}
 #define split_huge_pmd(__vma, __pmd, __address)        \
-       do { } while (0)
+       do { (void)__vma; } while (0)

 static inline void split_huge_pmd_address(struct vm_area_struct *vma,
                unsigned long address, bool freeze, struct page *page) {}
-- 
 Kirill A. Shutemov

[toc] | [prev] | [next] | [standalone]

#1538793

FromJérémy Lefaure <jeremy.lefaure@lse.epita.fr>
Date2016-12-08 19:30 +0100
Message-ID<sMbC9-11A-17@gated-at.bofh.it>
In reply to#1538429
On Thu, 8 Dec 2016 13:50:11 +0300
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote:

> On Wed, Dec 07, 2016 at 11:38:33PM -0500, Jérémy Lefaure wrote:
> > When CONFIG_TRANSPARENT_HUGEPAGE is disabled, split_huge_pmd is a no-op
> > stub. In such case, vma is unused and a compiler raises a warning:
> > 
> > arch/x86/kernel/vm86_32.c: In function ‘mark_screen_rdonly’:
> > arch/x86/kernel/vm86_32.c:180:26: warning: unused variable ‘vma’
> > [-Wunused-variable]
> >    struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> >                              ^~~
> > Adding __maybe_unused in the vma declaration fixes this warning.  
> 
> Hm. pmd_trans_huge() is zero if CONFIG_TRANSPARENT_HUGEPAGE is not set.
> Compiler should get rid of whole block of code under the 'if'.
> 
> Could you share your kernel config which triggers the warning?
> And what compiler do you use?
> 

After a `make allnoconfig`, I enable "Legacy VM86 support" and nothing
else. I tested with 2 compilers, gcc 4.9.2 (on debian jessie) and gcc
6.2.1 (on archlinux).

Actually, the compiler does not raise warnings on complete build (`make
mrproper`, configuration and `make`) but only on partial build (`make
arch/x86/kernel/vm86_32.o` or `touch arch/x86/kernel/vm86_32.c &&
make`). So maybe it is a compiler issue ?

The solution you propose in your other email (adding "(void)__vma;" in
the no-op split_huge_pmd) seems to fix the warnings on partial build.

Thanks,
Jérémy

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web