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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 18 Jan 2016 13:40:43 +0200
From:	"Kirill A. Shutemov" <kirill@...temov.name>
To:	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Hugh Dickins <hughd@...gle.com>,
	Dave Hansen <dave.hansen@...el.com>,
	Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Christoph Lameter <cl@...two.org>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	Steve Capper <steve.capper@...aro.org>,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Michal Hocko <mhocko@...e.cz>,
	Jerome Marchand <jmarchan@...hat.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Linux MM <linux-mm@...ck.org>
Subject: Re: BUILD_BUG() in smaps_account() (was: Re: [PATCHv12 01/37] mm,
 proc: adjust PSS calculation)

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@...ux.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@...ux.intel.com>
> > Tested-by: Sasha Levin <sasha.levin@...cle.com>
> > Tested-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.com>
> > Acked-by: Jerome Marchand <jmarchan@...hat.com>
> > Acked-by: Vlastimil Babka <vbabka@...e.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@...ux.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@...ux.intel.com>
Reported-by: Geert Uytterhoeven <geert@...ux-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