wi On Mon, Sep 13, 2021 at 7:13 PM Feng Tang <feng.tang@xxxxxxxxx> wrote: > > Hi Shakeel, > > On Mon, Sep 13, 2021 at 01:13:57PM -0700, Shakeel Butt wrote: > > On Mon, Sep 13, 2021 at 1:10 PM Tejun Heo <tj@xxxxxxxxxx> wrote: > > > > > > On Mon, Sep 13, 2021 at 01:09:11PM -0700, Shakeel Butt wrote: > > > > Thanks a lot for the explanation. Are there any concerns to call > > > > cgroup_rstat_flush_irqsafe(root_mem_cgroup->css.cgroup) in system_wq? > > > > This will be called every 2 seconds, so, we can assume the updated > > > > tree would be small most of the time. > > > > > > I can't think of a reason why this would be problematic. > > > > > > > Thanks again. > > > > Feng, is it possible to re-run these benchmarks with > > queue_work(system_wq) instead of queue_work(system_unbound_wq)? > > I just run the patch twice, and there was no obvious change, the > hotspot is still the spinlock. > > Thanks, > Feng > > $git-diff aa48e47e > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 4d8c9af..fa9cae9 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -683,7 +683,7 @@ void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx, > /* Update lruvec */ > __this_cpu_add(pn->lruvec_stats_percpu->state[idx], val); > if (!(__this_cpu_inc_return(stats_flush_threshold) % MEMCG_CHARGE_BATCH)) > - queue_work(system_unbound_wq, &stats_flush_work); > + queue_work(system_wq, &stats_flush_work); > } > > > 7e1c0d6f58207e7e aa48e47e3906c332eaf1e5d7b58 1638eee6432c1a5175685a7945a > ---------------- --------------------------- --------------------------- > \ | \ | \ > 648.62 +243.0% 2224 ± 3% +246.1% 2244 aim7.cpu > 588139 -13.4% 509421 -13.5% 508738 aim7.jobs-per-min > 196.05 -13.4% 169.81 -13.5% 169.58 aim7.jobs-per-min-per-task > 3.93 ± 3% +62.8 66.70 +63.4 67.37 pp.child.native_queued_spin_lock_slowpath > 3.96 ± 4% +62.8 66.76 +63.5 67.46 pp.child._raw_spin_lock_irqsave > 3.66 ± 5% +62.9 66.54 +63.6 67.22 pp.child.lock_page_lruvec_irqsave > 0.00 +0.1 0.10 ± 5% +0.1 0.09 pp.self.queue_work_on > 0.00 +0.2 0.18 ± 5% +0.2 0.22 pp.self.cgroup_rstat_flush_locked > 0.00 +0.6 0.60 ± 7% +0.7 0.72 ± 3% pp.self.mem_cgroup_css_rstat_flush > 0.19 +0.7 0.86 ± 5% +0.7 0.84 ± 4% pp.self.cgroup_rstat_updated > 3.92 ± 3% +62.8 66.70 +63.4 67.37 pp.self.native_queued_spin_lock_slowpath > > Thanks Feng, the spinlock in lock_page_lruvec_irqsave() is unrelated and not taken in __mod_memcg_lruvec_state(). If removing queue_work() from __mod_memcg_lruvec_state() removes the regression then I was expecting some improvement by replacing system_unbound_wq with system_wq. Anyways thanks for your help. I will run some benchmarks as well (the page fault ones from will-it-scale).
