On Fri, Oct 12, 2018 at 07:25:08PM -0700, Daniel Colascione wrote: [...] > > But anyway, I think this runtime detection thing is not needed. THP is > > actually expected to be as fast as this anyway, so if that's available then > > we should already be as fast. > > Ah, I think the commit message is confusing. (Or else I'm misreading > the patch now.) It's not quite that we're disabling the feature when > THP is enabled anywhere, but rather that we use the move_huge_pmd path > for huge PMDs and use the new code only for non-huge PMDs. (Right?) If > that's the case, the commit message shouldn't say "Incase THP is > enabled, the optimization is skipped". Even if THP is enabled on a > system generally, we might use the new PMD-moving code for mapping > types that don't support THP-ization, right?
That is true. Ok, I guess I can update the commit message to be more accurate about that. > > This is for non-THP where THP cannot be enabled > > and there is still room for some improvement. Most/all architectures will be > > just fine with this. This flag is more of a safety-net type of thing where > > in > > the future if there is this one or two weird architectures that don't play > > well, then they can turn it off at the architecture level by not selecting > > the flag. See my latest patches for the per-architecture compile-time > > controls. Ideally we'd like to blanket turn it on on all, but this is just > > playing it extra safe as Kirill and me were discussing on other threads. > > Sure. I'm just pointing out that the 500x performance different turns > the operation into a qualitatively different feature, so if we expect > to actually ship a mainstream architecture without support for this > thing, we should make it explicit. If we're not, we shouldn't. We can make it explicit by enabling it in such a mainstream architecture is my point. Also if the optimization is not doing what its supposed to, then userspace will also just know by measuring the time. thanks, - Joel
