[v2,0/2] dyndbg: let's use the module notifier callback

Message

Jason Baron Feb. 28, 2023, 7:34 p.m. UTC

Hi,

Jim Cromie hit a BUG() while toggling jump label branches in a module
before they were properly initialized. This isn't currently an issue,
but will be as part of his pending classmap series. Seems like we
should covert to using module callback notifier for dynamic debug
anyways. First patch is just a cleanup.

Link: https://lore.kernel.org/lkml/20230113193016.749791-21-jim.cromie@xxxxxxxxx/

Thanks,

-Jason

v2:
-Fix: error: field 'dyndbg_info' has incomplete type
 Reported-by: kernel test robot <[e-mail]>
 Link: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
-make ddebug_remove_module() static

Jason Baron (2):
  dyndbg: remove unused 'base' arg from __ddebug_add_module()
  dyndbg: use the module notifier callbacks

 include/linux/dynamic_debug.h | 13 ---------
 include/linux/module.h        |  4 +++
 kernel/module/internal.h      |  2 --
 kernel/module/main.c          | 30 ++++++---------------
 lib/dynamic_debug.c           | 51 ++++++++++++++++++++++++++++-------
 5 files changed, 53 insertions(+), 47 deletions(-)

Comments

Luis Chamberlain Feb. 28, 2023, 8:18 p.m. UTC | #1

On Tue, Feb 28, 2023 at 02:34:20PM -0500, Jason Baron wrote:
> Hi,
> 
> Jim Cromie hit a BUG() while toggling jump label branches in a module
> before they were properly initialized. This isn't currently an issue,
> but will be as part of his pending classmap series. Seems like we
> should covert to using module callback notifier for dynamic debug
> anyways. First patch is just a cleanup.
> 
> Link: https://lore.kernel.org/lkml/20230113193016.749791-21-jim.cromie@xxxxxxxxx/
> 
> Thanks,
> 
> -Jason
> 
> v2:
> -Fix: error: field 'dyndbg_info' has incomplete type
>  Reported-by: kernel test robot <[e-mail]>
>  Link: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
> -make ddebug_remove_module() static

Do you have tests to ensure no regressions have occurred? If so what
are they? If there are no tests, can you come up with some basic ones?

  Luis

Jason Baron Feb. 28, 2023, 8:28 p.m. UTC | #2

On 2/28/23 3:18 PM, Luis Chamberlain wrote:
> On Tue, Feb 28, 2023 at 02:34:20PM -0500, Jason Baron wrote:
>> Hi,
>>
>> Jim Cromie hit a BUG() while toggling jump label branches in a module
>> before they were properly initialized. This isn't currently an issue,
>> but will be as part of his pending classmap series. Seems like we
>> should covert to using module callback notifier for dynamic debug
>> anyways. First patch is just a cleanup.
>>
>> Link: https://urldefense.com/v3/__https://lore.kernel.org/lkml/20230113193016.749791-21-jim.cromie@xxxxxxxxx/__;!!GjvTz_vk!RcYAOcwdnCTVPf9WgS4GmmVwClimlsK1BfnLrRh4Oxlf1fJeOfBHa7zgCXDFiV8gw3YCGIbcdiYG$
>>
>> Thanks,
>>
>> -Jason
>>
>> v2:
>> -Fix: error: field 'dyndbg_info' has incomplete type
>>   Reported-by: kernel test robot <[e-mail]>
>>   Link: https://urldefense.com/v3/__https://lore.kernel.org/oe-kbuild-all/[e-mail]/__;!!GjvTz_vk!RcYAOcwdnCTVPf9WgS4GmmVwClimlsK1BfnLrRh4Oxlf1fJeOfBHa7zgCXDFiV8gw3YCGJKpcyr8$
>> -make ddebug_remove_module() static
> 
> Do you have tests to ensure no regressions have occurred? If so what
> are they? If there are no tests, can you come up with some basic ones?
> 
>    Luis


Hi Luis,

Yes, I've run the test case that prompted this series here:

https://lore.kernel.org/lkml/[e-mail]/

Note that this is for new functionality that Jim is working on and he 
plan to include this series when he re-posts his series. And thus I've 
added Jim's Tested-by: in patches 1 and 2.

There is also a 'test_dynamic_debug' module which I've run as follows:

# modprobe test_dynamic_debug dyndbg

And confirmed the appropriate prints to the log. This exercises the code 
paths that were changed here.

Thanks,

-Jason