On Sun, Jul 14, 2024 at 08:14:27AM +0300, botta633 wrote:
First, the subsystem tag also needs to be "locking/lockdep" or
"lockdep".
> Checking if the lockdep_map->name will change when setting the subclass.
> It shouldn't change so that the lock class and subclass will have the same name
>
Could you make the commit log wrapped at 75 columns?
>
> Reported-by: <syzbot+7f4a6f7f7051474e40ad@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Fixes: fd5e3f5fe27
> Cc: <stable@xxxxxxxxxxxxxxx>
Since this is only adding test for a bug fix, you don't need to put
these tags here.
> Signed-off-by: botta633 <bottaawesome633@xxxxxxxxx>
Again, could you please put your name here?
Also seems that you send two patch #2, one is with the proper version
number "v2", but not in-reply-to the patch #1, the other doesn't have
the correct version number but has the correct "in-reply-to" field.
Could you use the correct version number and "in-reply-to" next time?
> ---
> lib/locking-selftest.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
> index 6f6a5fc85b42..1d7885205f36 100644
> --- a/lib/locking-selftest.c
> +++ b/lib/locking-selftest.c
> @@ -2710,12 +2710,24 @@ static void local_lock_3B(void)
>
> }
>
> +static void class_subclass_X1_name(void)
> +{
> + const char *name_before_subclass = rwsem_X1.dep_map.name;
> + const char *name_after_subclass;
> +
> + WARN_ON(!rwsem_X1.dep_map.name);
> + lockdep_set_subclass(&rwsem_X1, 1);
> + WARN_ON(name_before_subclass != name_after_subclass);
Could you add some comments here explaining your test? For example,
where name_after_subclass gets set?
> +}
> +
> static void local_lock_tests(void)
> {
Please don't add this test into another test, you could directly call
your class_subclass_X1_name() (maybe rename it to *_test()) in
lockding_selftest() function.
Besides, make sure you run the test with and without your modification
in patch #1, and confirm this is the test that could verify your fix.
Regards,
Boqun
> printk(" --------------------------------------------------------------------------\n");
> printk(" | local_lock tests |\n");
> printk(" ---------------------\n");
>
> + init_class_X(&lock_X1, &rwlock_X1, &mutex_X1, &rwsem_X1);
> +
> print_testname("local_lock inversion 2");
> dotest(local_lock_2, SUCCESS, LOCKTYPE_LL);
> pr_cont("\n");
> @@ -2727,6 +2739,10 @@ static void local_lock_tests(void)
> print_testname("local_lock inversion 3B");
> dotest(local_lock_3B, FAILURE, LOCKTYPE_LL);
> pr_cont("\n");
> +
> + print_testname("Class and subclass");
> + dotest(class_subclass_X1_name, SUCCESS, LOCKTYPE_RWSEM);
> + pr_cont("\n");
> }
>
> static void hardirq_deadlock_softirq_not_deadlock(void)
> --
> 2.45.2
>
