Re: [PATCH v2 3/5] mm: enlarge NUMA counters threshold size

[Michal Hocko]

On Thu 21-12-17 18:31:19, kemi wrote:
> 
> 
> On 2017å12æ21æ 16:59, Michal Hocko wrote:
> > On Thu 21-12-17 16:23:23, kemi wrote:
> >>
> >>
> >> On 2017å12æ21æ 16:17, Michal Hocko wrote:
> > [...]
> >>> Can you see any difference with a more generic workload?
> >>>
> >>
> >> I didn't see obvious improvement for will-it-scale.page_fault1
> >> Two reasons for that:
> >> 1) too long code path
> >> 2) server zone lock and lru lock contention (access to buddy system frequently) 
> > 
> > OK. So does the patch helps for anything other than a microbenchmark?
> > 
> >>>> Some thinking about that:
> >>>> a) the overhead due to cache bouncing caused by NUMA counter update in fast path 
> >>>> severely increase with more and more CPUs cores
> >>>
> >>> What is an effect on a smaller system with fewer CPUs?
> >>>
> >>
> >> Several CPU cycles can be saved using single thread for that.
> >>
> >>>> b) AFAIK, the typical usage scenario (similar at least)for which this optimization can 
> >>>> benefit is 10/40G NIC used in high-speed data center network of cloud service providers.
> >>>
> >>> I would expect those would disable the numa accounting altogether.
> >>>
> >>
> >> Yes, but it is still worthy to do some optimization, isn't?
> > 
> > Ohh, I am not opposing optimizations but you should make sure that they
> > are worth the additional code and special casing. As I've said I am not
> > convinced special casing numa counters is good. You can play with the
> > threshold scaling for larger CPU count but let's make sure that the
> > benefit is really measurable for normal workloads. Special ones will
> > disable the numa accounting anyway.
> > 
> 
> I understood. Could you give me some suggestion for those normal workloads, Thanks.
> I will have a try and post the data ASAP. 

Well, to be honest, I am really confused what is your objective for
these optimizations then. I hope we have agreed that workloads which
really need to squeeze every single CPU cycle in the allocation path
will simply disable the whole numa stat thing. I haven't yet heard about
any use case which would really required numa stats and suffer from the
numa stats overhead.

I can see some arguments for a better threshold scaling but that
requires to check wider range of tests to show there are no unintended
changes. I am not really confident you understand that when you are
asking for "those normal workloads".

So please, try to step back, rethink who you are optimizing for and act
accordingly. If I were you I would repost the first patch which only
integrates numa stats because that removes a lot of pointless code and
that is a win of its own.

-- 
Michal Hocko
SUSE Labs