linux-kernel - Re: [PATCH 02/12] mm: introduce page_check

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]

Message-ID: <20170125175305.GB4157@node>
Date:   Wed, 25 Jan 2017 20:53:05 +0300
From:   "Kirill A. Shutemov" <kirill@...temov.name>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Hugh Dickins <hughd@...gle.com>,
        Rik van Riel <riel@...hat.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 02/12] mm: introduce page_check_walk()

On Tue, Jan 24, 2017 at 02:55:13PM -0800, Andrew Morton wrote:
> On Wed, 25 Jan 2017 01:50:30 +0300 "Kirill A. Shutemov" <kirill@...temov.name> wrote:
> 
> > > > + * @pcw->ptl is unlocked and @pcw->pte is unmapped.
> > > > + *
> > > > + * If you need to stop the walk before page_check_walk() returned false, use
> > > > + * page_check_walk_done(). It will do the housekeeping.
> > > > + */
> > > > +static inline bool page_check_walk(struct page_check_walk *pcw)
> > > > +{
> > > > +	/* The only possible pmd mapping has been handled on last iteration */
> > > > +	if (pcw->pmd && !pcw->pte) {
> > > > +		page_check_walk_done(pcw);
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	/* Only for THP, seek to next pte entry makes sense */
> > > > +	if (pcw->pte) {
> > > > +		if (!PageTransHuge(pcw->page) || PageHuge(pcw->page)) {
> > > > +			page_check_walk_done(pcw);
> > > > +			return false;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	return __page_check_walk(pcw);
> > > > +}
> > > 
> > > Was the decision to inline this a correct one?
> > 
> > Well, my logic was that in most cases we would have exactly one iteration.
> > The only case when we need more than one iteration is PTE-mapped THP which
> > is rare.
> > I hoped to avoid additional function call. Not sure if it worth it.
> > 
> > Should I move it inside the function?
> 
> I suggest building a kernel with it uninlined, take a look at the bloat
> factor then make a seat-of-the pants decision about "is it worth it". 
> With quite a few callsites the saving from uninlining may be
> significant.

add/remove: 1/2 grow/shrink: 8/0 up/down: 5089/-2954 (2135)
function                                     old     new   delta
__page_vma_mapped_walk                         -    2928   +2928
try_to_unmap_one                            2916    3218    +302
page_mkclean_one                             513     802    +289
__replace_page                              1439    1719    +280
page_referenced_one                          753    1030    +277
page_mapped_in_vma                           799    1059    +260
remove_migration_pte                        1129    1388    +259
page_idle_clear_pte_refs_one                 197     456    +259
write_protect_page                          1210    1445    +235
page_idle_clear_pte_refs_one.part             26       -     -26
page_vma_mapped_walk                        2928       -   -2928
Total: Before=37784555, After=37786690, chg +0.01%

I'll drop inlining. It would save ~2k.

-- 
 Kirill A. Shutemov