On Thu, Mar 02, 2023 at 11:47:35AM +0100, David Hildenbrand wrote: > On 09.02.23 16:01, Marcelo Tosatti wrote: > > In preparation to switch vmstat shepherd to flush > > per-CPU counters remotely, switch all functions that > > modify the counters to use cmpxchg. > > > > To test the performance difference, a page allocator microbenchmark: > > https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/mm/bench/page_bench01.c > > with loops=1000000 was used, on Intel Core i7-11850H @ 2.50GHz. > > > > For the single_page_alloc_free test, which does > > > > /** Loop to measure **/ > > for (i = 0; i < rec->loops; i++) { > > my_page = alloc_page(gfp_mask); > > if (unlikely(my_page == NULL)) > > return 0; > > __free_page(my_page); > > } > > > > Unit is cycles. > > > > Vanilla Patched Diff > > 159 165 3.7% > > > > Signed-off-by: Marcelo Tosatti <mtosatti@xxxxxxxxxx> > > > > Index: linux-vmstat-remote/mm/vmstat.c > > =================================================================== > > --- linux-vmstat-remote.orig/mm/vmstat.c > > +++ linux-vmstat-remote/mm/vmstat.c > > @@ -334,6 +334,188 @@ void set_pgdat_percpu_threshold(pg_data_ > > } > > } > > I wonder why we get a diff that is rather hard to review because it removes > all existing codes and replaces it by almost-identical code. Are you maybe > moving a bunch of code while modifying some tiny bits at the same time? Current code has functions defined like so: __mod_zone_page_state __mod_node_page_state __inc_zone_page_state __inc_node_page_state __dec_zone_page_state __dec_node_page_state #ifdef CONFIG_HAVE_CMPXCHG_LOCAL mod_zone_page_state inc_zone_page_state dec_zone_page_state mod_node_page_state inc_node_page_state dec_node_page_state #else mod_zone_page_state inc_zone_page_state dec_zone_page_state mod_node_page_state inc_node_page_state dec_node_page_state #endif What this patch is doing is to define the __ versions for the CONFIG_HAVE_CMPXCHG_LOCAL case to be their non-"__" counterparts. So it will be: #ifdef CONFIG_HAVE_CMPXCHG_LOCAL mod_zone_page_state inc_zone_page_state dec_zone_page_state mod_node_page_state inc_node_page_state dec_node_page_state __mod_zone_page_state (new function, calls mod_zone_page_state). __mod_node_page_state (new function, calls mod_node_page_state). __inc_zone_page_state __inc_node_page_state __dec_zone_page_state __dec_node_page_state #else __mod_zone_page_state (old, shared function for both CONFIG_HAVE_CMPXCHG_LOCAL and not) __mod_node_page_state __inc_zone_page_state __inc_node_page_state __dec_zone_page_state __dec_node_page_state mod_zone_page_state inc_zone_page_state dec_zone_page_state mod_node_page_state inc_node_page_state dec_node_page_state #endif Any suggestion on how to split this into multiple patchsets for easier reviewing? (can't think of anything obvious).
