On Thu, Aug 6, 2020 at 3:37 PM Uriel Guajardo <urielguajardojr@xxxxxxxxx> wrote: > > From: Uriel Guajardo <urielguajardo@xxxxxxxxxx> > > KUnit tests will now fail if lockdep detects an error during a test > case. > > The idea comes from how lib/locking-selftest [1] checks for lock errors: we > first if lock debugging is turned on. If not, an error must have > occurred, so we fail the test and restart lockdep for the next test case. > > Like the locking selftests, we also fix possible preemption count > corruption from lock bugs. > > Depends on kunit: support failure from dynamic analysis tools [2] > > [1] https://elixir.bootlin.com/linux/v5.7.12/source/lib/locking-selftest.c#L1137 > > [2] https://lore.kernel.org/linux-kselftest/20200806174326.3577537-1-urielguajardojr@xxxxxxxxx/ > > Signed-off-by: Uriel Guajardo <urielguajardo@xxxxxxxxxx> > --- > lib/kunit/test.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index d8189d827368..0838ececa005 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -11,6 +11,8 @@ > #include <linux/kref.h> > #include <linux/sched/debug.h> > #include <linux/sched.h> > +#include <linux/lockdep.h> > +#include <linux/debug_locks.h> > > #include "debugfs.h" > #include "string-stream.h" > @@ -22,6 +24,26 @@ void kunit_fail_current_test(void) > kunit_set_failure(current->kunit_test); > } > > +static inline void kunit_check_locking_bugs(struct kunit *test, > + unsigned long saved_preempt_count) > +{ > + preempt_count_set(saved_preempt_count); > +#ifdef CONFIG_TRACE_IRQFLAGS > + if (softirq_count()) > + current->softirqs_enabled = 0; > + else > + current->softirqs_enabled = 1; > +#endif I am not entirely sure why lib/locking-selftests enables/disables softirqs, but I suspect it has to do with the fact that preempt_count became corrupted, and somehow softirqs became incorrectly enabled/disabled as a result. The resetting of the preemption count will undo the enabling/disabling accordingly. Any insight on this would be appreciated! > +#if IS_ENABLED(CONFIG_LOCKDEP) > + local_irq_disable(); > + if (!debug_locks) { > + kunit_set_failure(test); > + lockdep_reset(); > + } > + local_irq_enable(); > +#endif > +} > + > static void kunit_print_tap_version(void) > { > static bool kunit_has_printed_tap_version; > @@ -289,6 +311,7 @@ static void kunit_try_run_case(void *data) > struct kunit *test = ctx->test; > struct kunit_suite *suite = ctx->suite; > struct kunit_case *test_case = ctx->test_case; > + unsigned long saved_preempt_count = preempt_count(); > > current->kunit_test = test; > > @@ -298,7 +321,8 @@ static void kunit_try_run_case(void *data) > * thread will resume control and handle any necessary clean up. > */ > kunit_run_case_internal(test, suite, test_case); > - /* This line may never be reached. */ > + /* These lines may never be reached. */ > + kunit_check_locking_bugs(test, saved_preempt_count); > kunit_run_case_cleanup(test, suite); > } > > -- > 2.28.0.236.gb10cc79966-goog >
