On Thu, Aug 6, 2020 at 1:43 PM Uriel Guajardo <urielguajardo@xxxxxxxxxx> wrote: > > 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. Sorry, just noticed: You probably want to send this to some of the lockdep maintainers or the maintainers of the kselftest for lockdep. > > 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 > >
