Hello, Zqiang. Thank you for your v4!
Some small nits see below: > From: Zqiang <[email protected]> > > Add free per-cpu existing krcp's page cache operation, when > the system is under memory pressure. > > Signed-off-by: Zqiang <[email protected]> > Co-developed-by: Uladzislau Rezki (Sony) <[email protected]> > --- > v1->v2->v3->v4: > During the test a page shrinker is pretty active, because of low memory > condition. callback drains it whereas kvfree_rcu() part refill it right > away making kind of vicious circle. > Through Vlad Rezki suggestion, to avoid this, schedule a periodic delayed > work with HZ, and it's easy to do that. > I think the commit message should be improved. Please add a clear description how it works, i mean its connection with shrinker, etc. > kernel/rcu/tree.c | 50 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 42 insertions(+), 8 deletions(-) > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index c1ae1e52f638..f3b772eef468 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -3139,7 +3139,7 @@ struct kfree_rcu_cpu { > bool initialized; > int count; > > - struct work_struct page_cache_work; > + struct delayed_work page_cache_work; > atomic_t work_in_progress; > struct hrtimer hrtimer; > > @@ -3395,7 +3395,7 @@ schedule_page_work_fn(struct hrtimer *t) > struct kfree_rcu_cpu *krcp = > container_of(t, struct kfree_rcu_cpu, hrtimer); > > - queue_work(system_highpri_wq, &krcp->page_cache_work); > + queue_delayed_work(system_highpri_wq, &krcp->page_cache_work, 0); > return HRTIMER_NORESTART; > } > > @@ -3404,7 +3404,7 @@ static void fill_page_cache_func(struct work_struct > *work) > struct kvfree_rcu_bulk_data *bnode; > struct kfree_rcu_cpu *krcp = > container_of(work, struct kfree_rcu_cpu, > - page_cache_work); > + page_cache_work.work); > unsigned long flags; > bool pushed; > int i; > @@ -3428,15 +3428,22 @@ static void fill_page_cache_func(struct work_struct > *work) > atomic_set(&krcp->work_in_progress, 0); > } > > +static bool backoff_page_cache_fill; > + > static void > run_page_cache_worker(struct kfree_rcu_cpu *krcp) > { > if (rcu_scheduler_active == RCU_SCHEDULER_RUNNING && > !atomic_xchg(&krcp->work_in_progress, 1)) { > - hrtimer_init(&krcp->hrtimer, CLOCK_MONOTONIC, > - HRTIMER_MODE_REL); > - krcp->hrtimer.function = schedule_page_work_fn; > - hrtimer_start(&krcp->hrtimer, 0, HRTIMER_MODE_REL); > + if (READ_ONCE(backoff_page_cache_fill)) { Can we just use xchg directly inside "if" statement? So we can get rid of below WRITE_ONCE(). It is not considered as a "hot" path, so it should not be an issue. > + queue_delayed_work(system_highpri_wq, > &krcp->page_cache_work, HZ); > + WRITE_ONCE(backoff_page_cache_fill, false); > + } else { > + hrtimer_init(&krcp->hrtimer, CLOCK_MONOTONIC, > + HRTIMER_MODE_REL); > + krcp->hrtimer.function = schedule_page_work_fn; > + hrtimer_start(&krcp->hrtimer, 0, HRTIMER_MODE_REL); > + } > } > } Thank you! -- Vlad Rezki
