Thread (34 messages) 34 messages, 7 authors, 2012-06-29

RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

From: Dan Magenheimer <hidden>
Date: 2012-06-27 21:16:08
Also in: lkml

From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
Sent: Wednesday, June 27, 2012 12:34 PM
To: Dan Magenheimer
Cc: Minchan Kim; Alex Shi; Greg Kroah-Hartman; devel@driverdev.osuosl.org; Konrad Wilk; linux-
kernel@vger.kernel.org; linux-mm@kvack.org; Andrew Morton; Robert Jennings; Nitin Gupta
Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
quoted
quoted
From: Minchan Kim [mailto:minchan@kernel.org]
Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

On 06/27/2012 03:14 PM, Alex Shi wrote:
quoted
On 06/27/2012 01:53 PM, Minchan Kim wrote:
Different CPU type has different balance point on the invlpg replacing
flush all. and some CPU never get benefit from invlpg, So, it's better
to use different value for different CPU, not a fixed
INVLPG_BREAK_EVEN_PAGES.
I think it could be another patch as further step and someone who are
very familiar with architecture could do better than.
So I hope it could be merged if it doesn't have real big problem.

Thanks for the comment, Alex.
Just my opinion, but I have to agree with Alex.  Hardcoding
behavior that is VERY processor-specific is a bad idea.  TLBs should
only be messed with when absolutely necessary, not for the
convenience of defending an abstraction that is nice-to-have
but, in current OS kernel code, unnecessary.
I agree that it's not optimal.  The selection based on CPUID
is part of Alex's patchset, and I'll be glad to use that
code when it gets integrated.

But the real discussion is are we going to:
1) wait until Alex's patches to be integrated, degrading
zsmalloc in the meantime or
2) put in some simple temporary logic that works well (not
best) for most cases
quoted
IIUC, zsmalloc only cares that the breakeven point is greater
than two.  An arch-specific choice of (A) two page flushes
vs (B) one all-TLB flush should be all that is necessary right
now.  (And, per separate discussion, even this isn't really
necessary either.)

If zsmalloc _ever_ gets extended to support items that might
span three or more pages, a more generic TLB flush-pages-vs-flush-all
approach may be warranted and, by then, may already exist in some
future kernel.  Until then, IMHO, keep it simple.
I guess I'm not following.  Are you supporting the removal
of the "break even" logic?  I added that logic as a
compromise for Peter's feedback:

http://lkml.org/lkml/2012/5/17/177
Yes, as long as I am correct that zsmalloc never has to map/flush
more than two pages at a time, I think dealing with the break-even
logic is overkill.  I see Peter isn't on this dist list... maybe
you should ask him if he agrees, as long as we are only always
talking about flush-two-TLB-pages vs flush-all.

(And, of course, per previous discussion, I think even mapping/flushing
two TLB pages is unnecessary and overkill required only for protecting an
abstraction, but will stop beating that dead horse. ;-)

Dan

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help