Messages in this thread | | | Date | Thu, 22 Jun 2017 21:05:22 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v3 06/11] x86/mm: Rework lazy TLB mode and TLB freshness tracking |
| |
On Thu, Jun 22, 2017 at 10:47:29AM -0700, Andy Lutomirski wrote: > I figured that some future reader of this patch might actually want to > see this text, though.
Oh, don't get me wrong: with commit messages more is more, in the general case. That's why I said "if".
> >> The UV tlbflush code is rather dated and should be changed. > > And I'd definitely like the UV maintainers to notice this part, now or > in the future :) I don't want to personally touch the UV code with a > ten-foot pole, but it really should be updated by someone who has a > chance of getting it right and being able to test it.
Ah, could be because they moved recently and have hpe addresses now. Lemme add them.
> >> + > >> + if (cpumask_test_cpu(cpu, mm_cpumask(mm))) > >> + cpumask_clear_cpu(cpu, mm_cpumask(mm)); > > > > It seems we haz a helper for that: cpumask_test_and_clear_cpu() which > > does BTR straightaway. > > Yeah, but I'm doing this for performance. I think that all the > various one-line helpers do a LOCKed op right away, and I think it's > faster to see if we can avoid the LOCKed op by trying an ordinary read > first.
Right, the test part of the operation is unlocked so if that is the likely case, it is a win.
> OTOH, maybe this is misguided -- if the cacheline lives somewhere else > and we do end up needing to update it, we'll end up first sharing it > and then making it exclusive, which increases the amount of cache > coherency traffic, so maybe I'm optimizing for the wrong thing. What > do you think?
Yeah, but we'll have to do that anyway for the locked operation. Ok, let's leave it split like it is.
> It did in one particular buggy incarnation. It would also trigger if, > say, suspend/resume corrupts CR3. Admittedly this is unlikely, but > I'd rather catch it. Once PCID is on, corruption seems a bit less > farfetched -- this assertion will catch anyone who accidentally does > write_cr3(read_cr3_pa()).
Ok, but let's put a comment over it pls as it is not obvious when something like that can happen.
-- Regards/Gruss, Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
| |