On Mon, Aug 10, 2020 at 3:35 PM Brendan Higgins <brendanhiggins@xxxxxxxxxx> wrote: > > 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. > Thanks for the reminder. CC'ed lockdep maintainers. > > > 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 > > >
