Date:   Wed, 7 Dec 2022 22:42:01 +0000
From:   Joel Fernandes <joelaf@...gle.com>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     paulmck@...nel.org, linux-kernel@...r.kernel.org,
        Ben Segall <bsegall@...gle.com>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Josh Triplett <josh@...htriplett.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Mel Gorman <mgorman@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Valentin Schneider <vschneid@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        kernel-team@...roid.com, John Stultz <jstultz@...gle.com>,
        Qais Yousef <qais.yousef@....com>,
        Will Deacon <will@...nel.org>,
        Waiman Long <longman@...hat.com>,
        Boqun Feng <boqun.feng@...il.com>,
        "Connor O'Brien" <connoro@...gle.com>
Subject: Re: [PATCH RFC 2/3] locktorture: Allow non-rtmutex lock types to be boosted

On Wed, Dec 7, 2022 at 10:23 PM Joel Fernandes <joel@...lfernandes.org> wrote:
>
> Hi Paul,
>
> On Wed, Dec 7, 2022 at 10:14 PM Paul E. McKenney <paulmck@...nel.org> wrote:
> >
> > On Wed, Nov 23, 2022 at 01:21:03AM +0000, Joel Fernandes (Google) wrote:
> > > Currently RT boosting is only done for rtmutex_lock, however with proxy
> > > execution, we also have the mutex_lock participating in priorities. To
> > > exercise the testing better, add RT boosting to other lock testing types
> > > as well, using a new knob (rt_boost).
> > >
> > > Tested with boot parameters:
> > > locktorture.torture_type=mutex_lock
> > > locktorture.onoff_interval=1
> > > locktorture.nwriters_stress=8
> > > locktorture.stutter=0
> > > locktorture.rt_boost=1
> > > locktorture.rt_boost_factor=1
> > > locktorture.nlocks=3
> > >
> > > For the rtmutex test, rt_boost is always enabled even if disabling is
> > > requested.
> > >
> > > Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> > > ---
> > >  kernel/locking/locktorture.c | 91 +++++++++++++++++++-----------------
> > >  1 file changed, 48 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> > > index bc3557677eed..5a388ac96a9b 100644
> > > --- a/kernel/locking/locktorture.c
> > > +++ b/kernel/locking/locktorture.c
> > > @@ -46,6 +46,7 @@ torture_param(int, shutdown_secs, 0, "Shutdown time (j), <= zero to disable.");
> > >  torture_param(int, stat_interval, 60,
> > >            "Number of seconds between stats printk()s");
> > >  torture_param(int, stutter, 5, "Number of jiffies to run/halt test, 0=disable");
> > > +torture_param(int, rt_boost, 0, "Perform an rt-boost from the writer, always 1 for rtmutex_lock");
> > >  torture_param(int, verbose, 1,
> > >            "Enable verbose debugging printk()s");
> > >  torture_param(int, nlocks, 1,
> > > @@ -129,15 +130,44 @@ static void torture_lock_busted_write_unlock(int tid __maybe_unused)
> > >         /* BUGGY, do not use in real life!!! */
> > >  }
> > >
> > > -static void torture_boost_dummy(struct torture_random_state *trsp)
> >
> > We no longer have torture_boot_dummy().  Is the point that the
> > "spinlocks" to priority boosting in PREEMPT_RT kernels?  If so,
> > would it make sense to do something like this for spinlock?
> >
> >         .task_boost     = IS_ENABLED(CONFIG_PREEMPT_RT) ? torture_rt_boost : torture_boost_dummy,
> >
> > Or maybe using a similar approach for the default value of the rt_boost
> > module parameter?
> >
> > Or is there some benefit of priority boosting for spinlocks even in
> > non-PREEMPT_RT kernels that I am missing?
>
> There are 2 advantages as far as I can see:
>
> 1. The shuffle thread which ends up in setscheduler exercises the same
> path as the rt mutex boost, so that would test races with that and the
> boost path.

I was too hasty in my reply, I meant here the thread's "RT boost"
racing with other paths such as migration / schedule() / other
setscheduler() paths.

Obviously rtmutex boost does not happen when there is no rtmutex to begin with.

Thanks,

  - Joel


>
> >
> >                                                         Thanx, Paul
> >
> > >       .writelock      = torture_rtmutex_lock,
> > >       .write_delay    = torture_rtmutex_delay,
> > > -     .task_boost     = torture_rtmutex_boost,
> > > +     .task_boost     = torture_rt_boost,
> > >       .writeunlock    = torture_rtmutex_unlock,
> > >       .readlock       = NULL,
> > >       .read_delay     = NULL,
> > > @@ -604,7 +609,7 @@ __releases(torture_rwsem)
> > >  static struct lock_torture_ops rwsem_lock_ops = {
> > >       .writelock      = torture_rwsem_down_write,
> > >       .write_delay    = torture_rwsem_write_delay,
> > > -     .task_boost     = torture_boost_dummy,
> > > +     .task_boost     = torture_rt_boost,
> > >       .writeunlock    = torture_rwsem_up_write,
> > >       .readlock       = torture_rwsem_down_read,
> > >       .read_delay     = torture_rwsem_read_delay,
> > > @@ -656,7 +661,7 @@ static struct lock_torture_ops percpu_rwsem_lock_ops = {
> > >       .exit           = torture_percpu_rwsem_exit,
> > >       .writelock      = torture_percpu_rwsem_down_write,
> > >       .write_delay    = torture_rwsem_write_delay,
> > > -     .task_boost     = torture_boost_dummy,
> > > +     .task_boost     = torture_rt_boost,
> > >       .writeunlock    = torture_percpu_rwsem_up_write,
> > >       .readlock       = torture_percpu_rwsem_down_read,
> > >       .read_delay     = torture_rwsem_read_delay,
> > > --
> > > 2.38.1.584.g0f3c55d4c2-goog
> > >

Powered by blists - more mailing lists