lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] rcu/kfree: Do not request RCU when not needed
    On Mon, Nov 14, 2022 at 7:20 AM Uladzislau Rezki <urezki@gmail.com> wrote:
    >
    > > On Thu, Nov 10, 2022 at 03:01:30PM +0100, Uladzislau Rezki wrote:
    > > > > Hi,
    > > > >
    > > > > On Thu, Nov 10, 2022 at 8:05 AM Uladzislau Rezki <urezki@gmail.com> wrote:
    > > > >
    > > > > > > On ChromeOS, using this with the increased timeout, we see that we
    > > > > > almost always
    > > > > > > never need to initiate a new grace period. Testing also shows this frees
    > > > > > large
    > > > > > > amounts of unreclaimed memory, under intense kfree_rcu() pressure.
    > > > > > >
    > > > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
    > > > > > > ---
    > > > > > > v1->v2: Same logic but use polled grace periods instead of sampling
    > > > > > gp_seq.
    > > > > > >
    > > > > > > kernel/rcu/tree.c | 8 +++++++-
    > > > > > > 1 file changed, 7 insertions(+), 1 deletion(-)
    > > > > > >
    > > > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
    > > > > > > index 591187b6352e..ed41243f7a49 100644
    > > > > > > --- a/kernel/rcu/tree.c
    > > > > > > +++ b/kernel/rcu/tree.c
    > > > > > > @@ -2935,6 +2935,7 @@ struct kfree_rcu_cpu_work {
    > > > > > >
    > > > > > > /**
    > > > > > > * struct kfree_rcu_cpu - batch up kfree_rcu() requests for RCU grace
    > > > > > period
    > > > > > > + * @gp_snap: The GP snapshot recorded at the last scheduling of monitor
    > > > > > work.
    > > > > > > * @head: List of kfree_rcu() objects not yet waiting for a grace period
    > > > > > > * @bkvhead: Bulk-List of kvfree_rcu() objects not yet waiting for a
    > > > > > grace period
    > > > > > > * @krw_arr: Array of batches of kfree_rcu() objects waiting for a
    > > > > > grace period
    > > > > > > @@ -2964,6 +2965,7 @@ struct kfree_rcu_cpu {
    > > > > > > struct kfree_rcu_cpu_work krw_arr[KFREE_N_BATCHES];
    > > > > > > raw_spinlock_t lock;
    > > > > > > struct delayed_work monitor_work;
    > > > > > > + unsigned long gp_snap;
    > > > > > > bool initialized;
    > > > > > > int count;
    > > > > > >
    > > > > > > @@ -3167,6 +3169,7 @@ schedule_delayed_monitor_work(struct kfree_rcu_cpu
    > > > > > *krcp)
    > > > > > > mod_delayed_work(system_wq, &krcp->monitor_work,
    > > > > > delay);
    > > > > > > return;
    > > > > > > }
    > > > > > > + krcp->gp_snap = get_state_synchronize_rcu();
    > > > > > > queue_delayed_work(system_wq, &krcp->monitor_work, delay);
    > > > > > > }
    > > > > > >
    > > > > > How do you guarantee a full grace period for objects which proceed
    > > > > > to be placed into an input stream that is not yet detached?
    > > > >
    > > > >
    > > > > Just replying from phone as I’m OOO today.
    > > > >
    > > > > Hmm, so you’re saying that objects can be queued after the delayed work has
    > > > > been queued, but processed when the delayed work is run? I’m looking at
    > > > > this code after few years so I may have missed something.
    > > > >
    > > > > That’s a good point and I think I missed that. I think your version did too
    > > > > but I’ll have to double check.
    > > > >
    > > > > The fix then is to sample the clock for the latest object queued, not for
    > > > > when the delayed work is queued.
    > > > >
    > > > The patch i sent gurantee it. Just in case see v2:
    > >
    > > You are right and thank you! CBs can be queued while the monitor timer is in
    > > progress. So we need to sample unconditionally. I think my approach is still
    > > better since I take advantage of multiple seconds (I update snapshot on every
    > > CB queue monitor and sample in the monitor handler).
    > >
    > > Whereas your patch is snapshotting before queuing the regular work and when
    > > the work is executed (This is a much shorter duration and I bet you would be
    > > blocking in cond_synchronize..() more often).
    > >
    > There is a performance test that measures a taken time and memory
    > footprint, so you can do a quick comparison. A "rcutorture" can be
    > run with various parameters to figure out if a patch that is in question
    > makes any difference.

    Yes sure, I am doing a run now with my patch. However, I have a
    question -- why do you feel blocking in the kworker is not an issue?
    You are taking a snapshot before queuing the normal kwork and then
    reading the snapshot when the normal kwork runs. Considering it is a
    high priority queue, the delay between when you are taking the
    snapshot, and reading it is likely small so there is a bigger chance
    of blocking in cond_synchronize_rcu(). Did I miss something?

    > Usually i run it as:
    >
    > <snip>
    > #! /usr/bin/env bash
    >
    > LOOPS=10
    >
    > for (( i=0; i<$LOOPS; i++ )); do
    > tools/testing/selftests/rcutorture/bin/kvm.sh --memory 10G --torture rcuscale --allcpus --duration 1 \
    > --kconfig CONFIG_NR_CPUS=64 \
    > --kconfig CONFIG_RCU_NOCB_CPU=y \
    > --kconfig CONFIG_RCU_NOCB_CPU_DEFAULT_ALL=y \
    > --kconfig CONFIG_RCU_LAZY=n \
    > --bootargs "rcuscale.kfree_rcu_test=1 rcuscale.kfree_nthreads=16 rcuscale.holdoff=20 rcuscale.kfree_loops=10000 torture.disable_onoff_at_boot" --trust-make
    > echo "Done $i"
    > done

    Sounds good, thanks.

    > <snip>
    >
    > just run it from your linux sandbox.

    Ok, will do.

    - Joel

    \
     
     \ /
      Last update: 2022-11-14 21:49    [W:4.505 / U:0.160 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site