linux-kernel - Re: [PATCH] x86,mm: delay TLB flush after clearing accessed bit

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:	Tue, 1 Apr 2014 15:20:37 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Rik van Riel <riel@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-mm@...ck.org, shli@...nel.org,
	akpm@...ux-foundation.org, hughd@...gle.com, mgorman@...e.de,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86,mm: delay TLB flush after clearing accessed bit


* Rik van Riel <riel@...hat.com> wrote:

> >>  int ptep_clear_flush_young(struct vm_area_struct *vma,
> >>  			   unsigned long address, pte_t *ptep)
> >>  {
> >> -	int young;
> >> +	int young, cpu;
> >>  
> >>  	young = ptep_test_and_clear_young(vma, address, ptep);
> >> -	if (young)
> >> -		flush_tlb_page(vma, address);
> >> +	if (young) {
> >> +		for_each_cpu(cpu, vma->vm_mm->cpu_vm_mask_var)
> >> +			tlb_set_force_flush(cpu);
> > 
> > Hm, just to play the devil's advocate - what happens when we have 
> > a va that is used on a few dozen, a few hundred or a few thousand 
> > CPUs? Will the savings be dwarved by the O(nr_cpus_used) loop 
> > overhead?
> > 
> > Especially as this is touching cachelines on other CPUs and likely 
> > creating the worst kind of cachemisses. That can really kill 
> > performance.
> 
> flush_tlb_page does the same O(nr_cpus_used) loop, but it sends an 
> IPI to each CPU every time, instead of dirtying a cache line once 
> per pageout run (or until the next context switch).
> 
> Does that address your concern?

That depends on the platform - which could implement flush_tlb_page() 
as a broadcast IPI - but yes, it was bad before as well, now it became 
more visible and I noticed it :)

Wouldn't it be more scalable to use a generation count as a timestamp, 
and set that in the mm? mm that last flushed before that timestamp 
need to flush, or so. That gets rid of the mask logic and the loop, 
AFAICS.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/