Re: [memcg] 45208c9105: aim7.jobs-per-min -14.0% regression

[Shakeel Butt]

On Sun, Sep 12, 2021 at 6:29 AM Feng Tang <feng.tang@xxxxxxxxx> wrote:
>
> On Sun, Sep 12, 2021 at 07:17:56PM +0800, Hillf Danton wrote:
> [...]
> > > +// if (!(__this_cpu_inc_return(stats_flush_threshold) % MEMCG_CHARGE_BATCH))
> > > +   if (!(__this_cpu_inc_return(stats_flush_threshold) % 128))
> > >             queue_work(system_unbound_wq, &stats_flush_work);
> > >  }
> >
> > Hi Feng,
> >
> > Would you please check if it helps fix the regression to avoid queuing a
> > queued work by adding and checking an atomic counter.
>
> Hi Hillf,
>
> I just tested your patch, and it didn't recover the regression, but
> just reduced it from -14% to around -13%, similar to the patch
> increasing the batch charge number.
>

Thanks Hillf for taking a look and Feng for running the test.

This shows that parallel calls to queue_work() is not the issue (there
is already a test and set at the start of queue_work()) but the actual
work done by queue_work() is costly for this code path.

I wrote a simple anon page fault nohuge c program, profiled it and it
seems like queue_work() is significant enough.

   - 51.00% do_anonymous_page
      + 16.68% alloc_pages_vma
        11.61% _raw_spin_lock
      + 10.26% mem_cgroup_charge
      - 5.25% lru_cache_add_inactive_or_unevictable
         - 4.48% __pagevec_lru_add
            - 3.71% __pagevec_lru_add_fn
               - 1.74% __mod_lruvec_state
                  - 1.60% __mod_memcg_lruvec_state
                     - 1.35% queue_work_on
                        - __queue_work
                           - 0.93% wake_up_process
                              - try_to_wake_up
                                 - 0.82% ttwu_queue
                                      0.61% ttwu_do_activate
      - 2.97% page_add_new_anon_rmap
         - 2.68% __mod_lruvec_page_state
            - 2.48% __mod_memcg_lruvec_state
               - 1.67% queue_work_on
                  - 1.53% __queue_work
                     - 1.25% wake_up_process
                        - try_to_wake_up
                           - 0.94% ttwu_queue
                              + 0.70% ttwu_do_activate
                 0.61% cgroup_rstat_updated
        2.10% rcu_read_unlock_strict
        1.40% cgroup_throttle_swaprate

However when I switch the batch size to 128, it goes away.

Feng, do you see any change with 128 batch size for aim7 benchmark?