MIME-Version: 1.0
References: <20230203200138.3872873-1-jstultz@google.com> <20230205183424.xwt54mlyixxgkhl6@offworld>
In-Reply-To: <20230205183424.xwt54mlyixxgkhl6@offworld>
From:   John Stultz <jstultz@google.com>
Date:   Mon, 6 Feb 2023 11:44:33 -0800
Message-ID: <CANDhNCqCahrCDRL2gg3c7E6vjY6cRThdDDE3=weTu2325ib3SQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/4] locktorture: Add nested_[un]lock() hooks and
 nlocks parameter
To:     Davidlohr Bueso <dave@stgolabs.net>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Josh Triplett <josh@joshtriplett.org>,
        Joel Fernandes <joel@joelfernandes.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Valentin Schneider <vschneid@redhat.com>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Feb 5, 2023 at 11:01 AM Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Fri, 03 Feb 2023, John Stultz wrote:
>
> >In order ot extend locktorture to support lock nesting, add
>           ^ to

Fixed. Thanks for catching this!

> >nested_lock() and nested_unlock() hooks to the torture ops.
> >
> >These take a 32bit lockset mask which is generated at random,
> >so some number of locks will be taken before the main lock is
> >taken and released afterwards.
> >
> >Additionally, add nlocks module parameter to allow specifying
> >the number of nested locks to be used.
> >
> >This has been helpful to uncover issues in the proxy-exec
> >series development.
> >
> >This was inspired by locktorture extensions originally implemented
> >by Connor O'Brien, for stress testing the proxy-execution series:
> >  https://lore.kernel.org/lkml/20221003214501.2050087-12-connoro@google.com/
> >
> >Comments or feedback would be greatly appreciated!
>
> Both of the above statements should be in a cover letter patch.

I'll drop the second line, but I'd like to preserve Connor's
contribution in the git log.
If it's ok, I'll keep it to just one of the patches to not be repetitive.


> >Cc: Davidlohr Bueso <dave@stgolabs.net>
> >Cc: "Paul E. McKenney" <paulmck@kernel.org>
> >Cc: Josh Triplett <josh@joshtriplett.org>
> >Cc: Joel Fernandes <joel@joelfernandes.org>
> >Cc: Juri Lelli <juri.lelli@redhat.com>
> >Cc: Valentin Schneider <vschneid@redhat.com>
> >Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>
> >Signed-off-by: John Stultz <jstultz@google.com>
>
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
>
> With some small nits below.
>
> >---
> > kernel/locking/locktorture.c | 23 +++++++++++++++++++----
> > 1 file changed, 19 insertions(+), 4 deletions(-)
> >
> >diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
> >index 9c2fb613a55d..f4fbd3194654 100644
> >--- a/kernel/locking/locktorture.c
> >+++ b/kernel/locking/locktorture.c
> >@@ -48,6 +48,9 @@ torture_param(int, stat_interval, 60,
> > torture_param(int, stutter, 5, "Number of jiffies to run/halt test, 0=disable");
> > torture_param(int, verbose, 1,
> >            "Enable verbose debugging printk()s");
> >+torture_param(int, nlocks, 0, "Number of nested locks");
>
> Maybe rename the parameter to 'nested_locks'? It would also be good to
> inform the 8 limit. "Number of nested locks (max = 8)".


Done!

Thanks so much for your time reviewing and the feedback!

I'll resend the reworked patches shortly.
-john