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


Groups > linux.kernel > #1311443 > unrolled thread

BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)

Started byGeert Uytterhoeven <geert@linux-m68k.org>
First post2016-01-18 11:10 +0100
Last post2016-01-21 20:40 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc:  adjust PSS calculation) Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-18 11:10 +0100
    Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm,  proc: adjust PSS calculation) "Kirill A. Shutemov" <kirill@shutemov.name> - 2016-01-18 12:50 +0100
      Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm,  proc: adjust PSS calculation) Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-18 16:00 +0100
        Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm,  proc: adjust PSS calculation) Tony Luck <tony.luck@gmail.com> - 2016-01-21 20:40 +0100

#1311443 — BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-01-18 11:10 +0100
SubjectBUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)
Message-ID<qSeV4-7C5-33@gated-at.bofh.it>
Hi Kirill,

On Tue, Oct 6, 2015 at 5:23 PM, Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
> With new refcounting all subpages of the compound page are not necessary
> have the same mapcount. We need to take into account mapcount of every
> sub-page.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Tested-by: Sasha Levin <sasha.levin@oracle.com>
> Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> Acked-by: Jerome Marchand <jmarchan@redhat.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  fs/proc/task_mmu.c | 47 +++++++++++++++++++++++++++++++----------------
>  1 file changed, 31 insertions(+), 16 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index bd167675a06f..ace02a4a07db 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -454,9 +454,10 @@ struct mem_size_stats {
>  };
>
>  static void smaps_account(struct mem_size_stats *mss, struct page *page,
> -               unsigned long size, bool young, bool dirty)
> +               bool compound, bool young, bool dirty)
>  {
> -       int mapcount;
> +       int i, nr = compound ? HPAGE_PMD_NR : 1;

If CONFIG_TRANSPARENT_HUGEPAGE is not set, we have:

    #define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
    #define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
    #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })

Depending on compiler version and optimization level, the BUILD_BUG() may be
optimized away (smaps_account() is always called with compound = false if
CONFIG_TRANSPARENT_HUGEPAGE=n), or lead to a build failure:

    fs/built-in.o: In function `smaps_account':
    task_mmu.c:(.text+0x4f8fa): undefined reference to
`__compiletime_assert_471'

Seen with m68k/allmodconfig or allyesconfig and gcc version 4.1.2 20061115
(prerelease) (Ubuntu 4.1.1-21).
Not seen when compiling the affected file with gcc 4.6.3 or 4.9.0, or with the
m68k defconfigs.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [next] | [standalone]


#1311496 — Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)

From"Kirill A. Shutemov" <kirill@shutemov.name>
Date2016-01-18 12:50 +0100
SubjectRe: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)
Message-ID<qSgtP-8vA-5@gated-at.bofh.it>
In reply to#1311443
On Mon, Jan 18, 2016 at 11:09:00AM +0100, Geert Uytterhoeven wrote:
> Hi Kirill,
> 
> On Tue, Oct 6, 2015 at 5:23 PM, Kirill A. Shutemov
> <kirill.shutemov@linux.intel.com> wrote:
> > With new refcounting all subpages of the compound page are not necessary
> > have the same mapcount. We need to take into account mapcount of every
> > sub-page.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Tested-by: Sasha Levin <sasha.levin@oracle.com>
> > Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > Acked-by: Jerome Marchand <jmarchan@redhat.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > ---
> >  fs/proc/task_mmu.c | 47 +++++++++++++++++++++++++++++++----------------
> >  1 file changed, 31 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index bd167675a06f..ace02a4a07db 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -454,9 +454,10 @@ struct mem_size_stats {
> >  };
> >
> >  static void smaps_account(struct mem_size_stats *mss, struct page *page,
> > -               unsigned long size, bool young, bool dirty)
> > +               bool compound, bool young, bool dirty)
> >  {
> > -       int mapcount;
> > +       int i, nr = compound ? HPAGE_PMD_NR : 1;
> 
> If CONFIG_TRANSPARENT_HUGEPAGE is not set, we have:
> 
>     #define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
>     #define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
>     #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
> 
> Depending on compiler version and optimization level, the BUILD_BUG() may be
> optimized away (smaps_account() is always called with compound = false if
> CONFIG_TRANSPARENT_HUGEPAGE=n), or lead to a build failure:
> 
>     fs/built-in.o: In function `smaps_account':
>     task_mmu.c:(.text+0x4f8fa): undefined reference to
> `__compiletime_assert_471'
> 
> Seen with m68k/allmodconfig or allyesconfig and gcc version 4.1.2 20061115
> (prerelease) (Ubuntu 4.1.1-21).
> Not seen when compiling the affected file with gcc 4.6.3 or 4.9.0, or with the
> m68k defconfigs.

Ughh.

Please, test this:

From 5ac27019f886eef033e84c9579e09099469ccbf9 Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Date: Mon, 18 Jan 2016 14:32:49 +0300
Subject: [PATCH] mm, proc: add workaround for old compilers

For THP=n, HPAGE_PMD_NR in smaps_account() expands to BUILD_BUG().
That's fine since this codepath is eliminated by modern compilers.

But older compilers have not that efficient dead code elimination.
It causes problem at least with gcc 4.1.2 on m68k.

Let's replace HPAGE_PMD_NR with 1 << compound_order(page).

Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 fs/proc/task_mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 65a1b6c69c11..71ffc91060f6 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -468,7 +468,7 @@ struct mem_size_stats {
 static void smaps_account(struct mem_size_stats *mss, struct page *page,
 		bool compound, bool young, bool dirty)
 {
-	int i, nr = compound ? HPAGE_PMD_NR : 1;
+	int i, nr = compound ? 1 << compound_order(page) : 1;
 	unsigned long size = nr * PAGE_SIZE;
 
 	if (PageAnon(page))
-- 
 Kirill A. Shutemov

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


#1311595 — Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-01-18 16:00 +0100
SubjectRe: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)
Message-ID<qSjrI-23m-11@gated-at.bofh.it>
In reply to#1311496
Hi Kirill,

On Mon, Jan 18, 2016 at 12:40 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> On Mon, Jan 18, 2016 at 11:09:00AM +0100, Geert Uytterhoeven wrote:
>>     fs/built-in.o: In function `smaps_account':
>>     task_mmu.c:(.text+0x4f8fa): undefined reference to
>> `__compiletime_assert_471'
>>
>> Seen with m68k/allmodconfig or allyesconfig and gcc version 4.1.2 20061115
>> (prerelease) (Ubuntu 4.1.1-21).
>> Not seen when compiling the affected file with gcc 4.6.3 or 4.9.0, or with the
>> m68k defconfigs.
>
> Ughh.
>
> Please, test this:
>
> From 5ac27019f886eef033e84c9579e09099469ccbf9 Mon Sep 17 00:00:00 2001
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Date: Mon, 18 Jan 2016 14:32:49 +0300
> Subject: [PATCH] mm, proc: add workaround for old compilers
>
> For THP=n, HPAGE_PMD_NR in smaps_account() expands to BUILD_BUG().
> That's fine since this codepath is eliminated by modern compilers.
>
> But older compilers have not that efficient dead code elimination.
> It causes problem at least with gcc 4.1.2 on m68k.
>
> Let's replace HPAGE_PMD_NR with 1 << compound_order(page).
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks, that fixes it!

Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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


#1314463 — Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)

FromTony Luck <tony.luck@gmail.com>
Date2016-01-21 20:40 +0100
SubjectRe: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm, proc: adjust PSS calculation)
Message-ID<qTtfl-1mO-27@gated-at.bofh.it>
In reply to#1311595
On Mon, Jan 18, 2016 at 6:56 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Kirill,
>
> On Mon, Jan 18, 2016 at 12:40 PM, Kirill A. Shutemov
> <kirill@shutemov.name> wrote:
>> On Mon, Jan 18, 2016 at 11:09:00AM +0100, Geert Uytterhoeven wrote:
>>>     fs/built-in.o: In function `smaps_account':
>>>     task_mmu.c:(.text+0x4f8fa): undefined reference to
>>> `__compiletime_assert_471'
>>>
>>> Seen with m68k/allmodconfig or allyesconfig and gcc version 4.1.2 20061115
>>> (prerelease) (Ubuntu 4.1.1-21).
>>> Not seen when compiling the affected file with gcc 4.6.3 or 4.9.0, or with the
>>> m68k defconfigs.
>>
>> Ughh.
>>
>> Please, test this:
>>
>> From 5ac27019f886eef033e84c9579e09099469ccbf9 Mon Sep 17 00:00:00 2001
>> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>> Date: Mon, 18 Jan 2016 14:32:49 +0300
>> Subject: [PATCH] mm, proc: add workaround for old compilers
>>
>> For THP=n, HPAGE_PMD_NR in smaps_account() expands to BUILD_BUG().
>> That's fine since this codepath is eliminated by modern compilers.
>>
>> But older compilers have not that efficient dead code elimination.
>> It causes problem at least with gcc 4.1.2 on m68k.
>>
>> Let's replace HPAGE_PMD_NR with 1 << compound_order(page).
>>
>> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Thanks, that fixes it!
>
> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>

Same breakage on ia64 (with gcc 4.3.4).  Same fix works for me.

Tested-by: Tony Luck <tony.luck@intel.com>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web