| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 12 Mar 2021 06:49:12 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Uladzislau Rezki <urezki@...il.com>
Cc: Michal Hocko <mhocko@...e.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Daniel Axtens <dja@...ens.net>,
Frederic Weisbecker <frederic@...nel.org>,
Neeraj Upadhyay <neeraju@...eaurora.org>,
Joel Fernandes <joel@...lfernandes.org>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Theodore Y . Ts'o" <tytso@....edu>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
Oleksiy Avramchenko <oleksiy.avramchenko@...ymobile.com>,
RCU <rcu@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] kvfree_rcu: convert a page cache to lock-free variant
On Fri, Mar 12, 2021 at 01:38:14PM +0100, Uladzislau Rezki wrote:
> Hello, Paul.
>
> Please do not consider this patch. It is buggy :)
Consider it formally withdrawn, and thank you for letting me know!
Thanx, Paul
> --
> Vlad Rezki
>
> On Mon, Mar 8, 2021 at 8:50 PM Uladzislau Rezki (Sony) <urezki@...il.com>
> wrote:
>
> > Implement an access to the page cache as lock-free variant. This
> > is done because there are extra places where an access is required,
> > therefore making it lock-less will remove any lock contention.
> >
> > For example we have a shrinker path as well as a reclaim kthread.
> > In both cases a current CPU can access to a remote per-cpu page
> > cache that would require taking a lock to protect it.
> >
> > An "rcuscale" performance test suite can detect it and shows some
> > slight improvements:
> >
> > ../kvm.sh --memory 16G --torture rcuscale --allcpus --duration 10 \
> > --kconfig CONFIG_NR_CPUS=64 --bootargs "rcuscale.kfree_rcu_test=1 \
> > rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 rcuscale.kfree_loops=10000 \
> > rcuscale.kfree_rcu_test_double=1 torture.disable_onoff_at_boot"
> > --trust-make
> >
> > 100 iterations, checking total time taken by all kfree'ers in ns.:
> >
> > default: AVG: 10968415107.5 MIN: 10668412500 MAX: 11312145160
> > patch: AVG: 10787596486.1 MIN: 10397559880 MAX: 11214901050
> >
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@...il.com>
> > ---
> > kernel/rcu/tree.c | 91 +++++++++++++++++++++++++++++------------------
> > 1 file changed, 56 insertions(+), 35 deletions(-)
> >
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 9c8cfb01e9a6..4f04664d5ac0 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -3167,8 +3167,9 @@ struct kfree_rcu_cpu {
> > atomic_t work_in_progress;
> > struct hrtimer hrtimer;
> >
> > + // lock-free cache.
> > struct llist_head bkvcache;
> > - int nr_bkv_objs;
> > + atomic_t nr_bkv_objs;
> > };
> >
> > static DEFINE_PER_CPU(struct kfree_rcu_cpu, krc) = {
> > @@ -3215,49 +3216,79 @@ krc_this_cpu_unlock(struct kfree_rcu_cpu *krcp,
> > unsigned long flags)
> > raw_spin_unlock_irqrestore(&krcp->lock, flags);
> > }
> >
> > +/*
> > + * Increment 'v', if 'v' is below 'thresh'. Returns true if we
> > + * succeeded, false if 'v' + 1 would be bigger than 'thresh'.
> > + *
> > + * Decrement 'v' if 'v' is upper 'thresh'. Returns true if we
> > + * succeeded, false if 'v' - 1 would be smaller than 'thresh'.
> > + */
> > +static inline bool
> > +atomic_test_inc_dec(atomic_t *v, unsigned int thresh, bool inc)
> > +{
> > + unsigned int cur = atomic_read(v);
> > + unsigned int old;
> > +
> > + for (;;) {
> > + if (inc) {
> > + if (cur >= thresh)
> > + return false;
> > + } else {
> > + if (cur <= thresh)
> > + return false;
> > + }
> > +
> > + old = atomic_cmpxchg(v, cur, inc ? (cur + 1):(cur - 1));
> > + if (old == cur)
> > + break;
> > +
> > + cur = old;
> > + }
> > +
> > + return true;
> > +}
> > +
> > static inline struct kvfree_rcu_bulk_data *
> > get_cached_bnode(struct kfree_rcu_cpu *krcp)
> > {
> > - if (!krcp->nr_bkv_objs)
> > - return NULL;
> > + struct kvfree_rcu_bulk_data *bnode = NULL;
> >
> > - krcp->nr_bkv_objs--;
> > - return (struct kvfree_rcu_bulk_data *)
> > - llist_del_first(&krcp->bkvcache);
> > + if (atomic_test_inc_dec(&krcp->nr_bkv_objs, 0, false))
> > + bnode = (struct kvfree_rcu_bulk_data *)
> > + llist_del_first(&krcp->bkvcache);
> > +
> > + return bnode;
> > }
> >
> > static inline bool
> > put_cached_bnode(struct kfree_rcu_cpu *krcp,
> > struct kvfree_rcu_bulk_data *bnode)
> > {
> > - // Check the limit.
> > - if (krcp->nr_bkv_objs >= rcu_min_cached_objs)
> > - return false;
> > -
> > - llist_add((struct llist_node *) bnode, &krcp->bkvcache);
> > - krcp->nr_bkv_objs++;
> > - return true;
> > + if (atomic_test_inc_dec(&krcp->nr_bkv_objs, rcu_min_cached_objs,
> > true)) {
> > + llist_add((struct llist_node *) bnode, &krcp->bkvcache);
> > + return true;
> > + }
> >
> > + return false;
> > }
> >
> > static int
> > drain_page_cache(struct kfree_rcu_cpu *krcp)
> > {
> > - unsigned long flags;
> > - struct llist_node *page_list, *pos, *n;
> > - int freed = 0;
> > + struct kvfree_rcu_bulk_data *bnode;
> > + int num_pages, i;
> >
> > - raw_spin_lock_irqsave(&krcp->lock, flags);
> > - page_list = llist_del_all(&krcp->bkvcache);
> > - krcp->nr_bkv_objs = 0;
> > - raw_spin_unlock_irqrestore(&krcp->lock, flags);
> > + num_pages = atomic_read(&krcp->nr_bkv_objs);
> > +
> > + for (i = 0; i < num_pages; i++) {
> > + bnode = get_cached_bnode(krcp);
> > + if (!bnode)
> > + break;
> >
> > - llist_for_each_safe(pos, n, page_list) {
> > - free_page((unsigned long)pos);
> > - freed++;
> > + free_page((unsigned long) bnode);
> > }
> >
> > - return freed;
> > + return i;
> > }
> >
> > /*
> > @@ -3314,10 +3345,8 @@ static void kfree_rcu_work(struct work_struct *work)
> > }
> > rcu_lock_release(&rcu_callback_map);
> >
> > - raw_spin_lock_irqsave(&krcp->lock, flags);
> > if (put_cached_bnode(krcp, bkvhead[i]))
> > bkvhead[i] = NULL;
> > - raw_spin_unlock_irqrestore(&krcp->lock, flags);
> >
> > if (bkvhead[i])
> > free_page((unsigned long) bkvhead[i]);
> > @@ -3460,7 +3489,6 @@ static void fill_page_cache_func(struct work_struct
> > *work)
> > struct kfree_rcu_cpu *krcp =
> > container_of(work, struct kfree_rcu_cpu,
> > page_cache_work.work);
> > - unsigned long flags;
> > bool pushed;
> > int i;
> >
> > @@ -3469,10 +3497,7 @@ static void fill_page_cache_func(struct work_struct
> > *work)
> > __get_free_page(GFP_KERNEL | __GFP_NORETRY |
> > __GFP_NOMEMALLOC | __GFP_NOWARN);
> >
> > if (bnode) {
> > - raw_spin_lock_irqsave(&krcp->lock, flags);
> > pushed = put_cached_bnode(krcp, bnode);
> > - raw_spin_unlock_irqrestore(&krcp->lock, flags);
> > -
> > if (!pushed) {
> > free_page((unsigned long) bnode);
> > break;
> > @@ -3647,17 +3672,13 @@ kfree_rcu_shrink_count(struct shrinker *shrink,
> > struct shrink_control *sc)
> > {
> > int cpu;
> > unsigned long count = 0;
> > - unsigned long flags;
> >
> > /* Snapshot count of all CPUs */
> > for_each_possible_cpu(cpu) {
> > struct kfree_rcu_cpu *krcp = per_cpu_ptr(&krc, cpu);
> >
> > count += READ_ONCE(krcp->count);
> > -
> > - raw_spin_lock_irqsave(&krcp->lock, flags);
> > - count += krcp->nr_bkv_objs;
> > - raw_spin_unlock_irqrestore(&krcp->lock, flags);
> > + count += atomic_read(&krcp->nr_bkv_objs);
> > }
> >
> > WRITE_ONCE(backoff_page_cache_fill, true);
> > --
> > 2.20.1
> >
> >
>
> --
> Uladzislau Rezki
Powered by blists - more mailing lists