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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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)
--
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
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
On Thu, 8 Dec 2016 09:33:05 +0100
Borislav Petkov <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Thu, 8 Dec 2016 13:50:11 +0300
"Kirill A. Shutemov" <[email protected]> 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
On Thu, Dec 08, 2016 at 01:25:37PM -0500, Jérémy Lefaure wrote:
> On Thu, 8 Dec 2016 13:50:11 +0300
> "Kirill A. Shutemov" <[email protected]> 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.
This doesn't make any sense, but this works too:
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index e339717af265..63ddc598f5a9 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -160,6 +160,7 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval)
static void mark_screen_rdonly(struct mm_struct *mm)
{
+ struct vm_area_struct *vma;
pgd_t *pgd;
p4d_t *p4d;
pud_t *pud;
@@ -181,7 +182,7 @@ static void mark_screen_rdonly(struct mm_struct *mm)
pmd = pmd_offset(pud, 0xA0000);
if (pmd_trans_huge(*pmd)) {
- struct vm_area_struct *vma = find_vma(mm, 0xA0000);
+ vma = find_vma(mm, 0xA0000);
split_huge_pmd(vma, pmd, 0xA0000);
}
if (pmd_none_or_clear_bad(pmd))
--
Kirill A. Shutemov
On Mon, 12 Dec 2016 17:52:50 +0300
"Kirill A. Shutemov" <[email protected]> wrote:
> On Thu, Dec 08, 2016 at 01:25:37PM -0500, Jérémy Lefaure wrote:
> > On Thu, 8 Dec 2016 13:50:11 +0300
> > "Kirill A. Shutemov" <[email protected]> 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 ?
> >
Sorry, forget about this part, it's false. I may have tested too
quickly and missed something.
> > 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.
>
> This doesn't make any sense, but this works too:
>
I don't know why gcc raises a warning on that even if it is not used.
Anyway, I'm sure that the warning is reproducible. Both of your
solutions fix the issue.
> diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
> index e339717af265..63ddc598f5a9 100644
> --- a/arch/x86/kernel/vm86_32.c
> +++ b/arch/x86/kernel/vm86_32.c
> @@ -160,6 +160,7 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval)
>
> static void mark_screen_rdonly(struct mm_struct *mm)
> {
> + struct vm_area_struct *vma;
> pgd_t *pgd;
> p4d_t *p4d;
> pud_t *pud;
> @@ -181,7 +182,7 @@ static void mark_screen_rdonly(struct mm_struct *mm)
> pmd = pmd_offset(pud, 0xA0000);
>
> if (pmd_trans_huge(*pmd)) {
> - struct vm_area_struct *vma = find_vma(mm, 0xA0000);
> + vma = find_vma(mm, 0xA0000);
> split_huge_pmd(vma, pmd, 0xA0000);
> }
> if (pmd_none_or_clear_bad(pmd))