linux-kernel - Re: [PATCH, RFC] x86/mm/pat: Restore large pages after fragmentation

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: <20200417165451.y2mj7ta3rqtxjcdc@box>
Date:   Fri, 17 Apr 2020 19:54:51 +0300
From:   "Kirill A. Shutemov" <kirill@...temov.name>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH, RFC] x86/mm/pat: Restore large pages after fragmentation

On Fri, Apr 17, 2020 at 05:47:14PM +0200, Peter Zijlstra wrote:
> On Fri, Apr 17, 2020 at 12:32:29AM +0300, Kirill A. Shutemov wrote:
> > +static void cpa_restore_large_pages(struct cpa_data *cpa,
> > +		struct list_head *pgtables)
> > +{
> > +	unsigned long start, addr, end;
> > +	int i;
> > +
> 
> > +	start = __cpa_addr(cpa, 0);
> > +	end = start + PAGE_SIZE * cpa->numpages;
> > +
> > +	for (addr = start; addr >= start && addr < end; addr += PUD_SIZE)
> > +		restore_large_pages(addr, pgtables);
> 
> Isn't that loop slightly broken?
> 
> Consider:
> 
> 	         s                     e
> 	|---------|---------|---------|---------|
>                  a0        a1        a2        a3
> 
> Where s,e are @start,@end resp. and a# are the consecutive values of
> @addr with PUD sized steps.
> 
> Then, since a3 is >= @end, we'll not take that iteration and we'll not
> try and merge that last PUD, even though we possibly could. One fix is
> to truncate @start (and with that @addr) to the beginning of the PUD.

... or round_up() end. I'll fix it.

> Also, I'm afraid that with my proposal this loop needs to do PMD size
> steps. In that regard your version does make some sense. But it is
> indeed less efficient for small ranges.
> 
> One possible fix is to pass @start,@end into the
> restore/reconstruct/collapse such that we can iterate the minimal set of
> page-tables for each level.

Yeah, I'll rework it.

I just realized I missed TLB flush: we need to flush TLB twice here. First
to get rid of all TLB entires for change we've made (before
reconstruction) and then the second time to get rid of small page TLB
entries. That's unfortunate.

-- 
 Kirill A. Shutemov