Hi Ahmed, On Thu, Sep 05, 2024 at 04:12:20AM +0300, Ahmed Ehab wrote: > Add a test case to ensure that no new name string literal will be > created in lockdep_set_subclass(), otherwise a warning will be triggered > in look_up_lock_class(). Add this to catch the problem in the future. > This overall looks good to me, I'm going to take it and create a PR for tip in next release. However, please note a few things: * Since you only send one of the patch from your original series, you should avoid use "2/2" in the title, because it could be confusing whether there is "1/2" lost in sending. If you want to make sure people aware that this is a continued work of the patch #2 in your original series, you can put some description after the following "---" * You need also to put changes between patch versions after "---" so that people can know the context, for example, I have no idea why you send a v8 after v7 and what's the delta here. Here is an example of how to document the delta: https://lore.kernel.org/rust-for-linux/20240827-static-mutex-v2-1-17fc32b20332@xxxxxxxxxx/ Regards, Boqun > Signed-off-by: Ahmed Ehab <bottaawesome633@xxxxxxxxx> > --- > lib/locking-selftest.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) > > diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c > index 6f6a5fc85b42..0783ee97c971 100644 > --- a/lib/locking-selftest.c > +++ b/lib/locking-selftest.c > @@ -2710,6 +2710,44 @@ static void local_lock_3B(void) > > } > > +#if CONFIG_DEBUG_LOCK_ALLOC > +static inline const char *rw_semaphore_lockdep_name(struct rw_semaphore *rwsem) > +{ > + return rwsem->dep_map.name; > +} > +#else > +static inline const char *rw_semaphore_lockdep_name(struct rw_semaphore *rwsem) > +{ > + return NULL; > +} > +#endif > + > +static void lock_class_subclass_X1(void) > +{ > + const char *name_before_setting_subclass = rw_semaphore_lockdep_name(&rwsem_X1); > + const char *name_after_setting_subclass; > + > + lockdep_set_subclass(&rwsem_X1, 1); > + name_after_setting_subclass = rw_semaphore_lockdep_name(&rwsem_X1); > + DEBUG_LOCKS_WARN_ON(name_before_setting_subclass != name_after_setting_subclass); > +} > + > +/* > + * after setting the subclass the lockdep_map.name changes > + * if we initialize a new string literal for the subclass > + * we will have a new name pointer > + */ > +static void class_subclass_X1_name_test(void) > +{ > + printk(" --------------------------------------------------------------------------\n"); > + printk(" | class and subclass name test|\n"); > + printk(" ---------------------\n"); > + > + print_testname("lock class and subclass same name"); > + dotest(lock_class_subclass_X1, SUCCESS, LOCKTYPE_RWSEM); > + pr_cont("\n"); > +} > + > static void local_lock_tests(void) > { > printk(" --------------------------------------------------------------------------\n"); > @@ -2920,6 +2958,8 @@ void locking_selftest(void) > dotest(hardirq_deadlock_softirq_not_deadlock, FAILURE, LOCKTYPE_SPECIAL); > pr_cont("\n"); > > + class_subclass_X1_name_test(); > + > if (unexpected_testcase_failures) { > printk("-----------------------------------------------------------------\n"); > debug_locks = 0; > -- > 2.46.0 >
