Re: [PATCH v2 03/11] staging: mt7621-mmc: Fix warning when reloading module with debug msgs enabled

From: Greg Kroah-Hartman
Date: Wed Mar 20 2019 - 03:20:07 EST


On Mon, Mar 18, 2019 at 08:20:04PM -0600, George Hilliard wrote:
> The kernel complained:
>
> [ 510.277151] WARNING: CPU: 0 PID: 395 at fs/proc/generic.c:360 proc_register+0xf0/0x108
> [ 510.292891] proc_dir_entry '/proc/msdc_debug' already registered
>
> when doing a modprobe/rmmod/modprobe of this module if debug messages
> are compiled in. Fix this by removing the proc entry when the module is
> unloaded.
>
> Signed-off-by: George Hilliard <thirtythreeforty@xxxxxxxxx>
> ---
> drivers/staging/mt7621-mmc/dbg.c | 15 ++++++++++++---
> drivers/staging/mt7621-mmc/dbg.h | 3 ++-
> drivers/staging/mt7621-mmc/sd.c | 4 ++++
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-mmc/dbg.c b/drivers/staging/mt7621-mmc/dbg.c
> index fda3ba38ba37..2310f3bcc16e 100644
> --- a/drivers/staging/mt7621-mmc/dbg.c
> +++ b/drivers/staging/mt7621-mmc/dbg.c
> @@ -294,9 +294,18 @@ static const struct file_operations msdc_debug_fops = {
> .release = single_release,
> };
>
> -void msdc_debug_proc_init(void)
> +// Keep ahold of the proc entry we create so it can be freed during
> +// module removal
> +struct proc_dir_entry *msdc_debug_proc_entry;
> +
> +void __init msdc_debug_proc_init(void)
> {
> - proc_create("msdc_debug", 0660, NULL, &msdc_debug_fops);
> + msdc_debug_proc_entry = proc_create("msdc_debug", 0660,
> + NULL, &msdc_debug_fops);
> +}
> +
> +void __exit msdc_debug_proc_deinit(void)
> +{
> + proc_remove(msdc_debug_proc_entry);
> }
> -EXPORT_SYMBOL_GPL(msdc_debug_proc_init);
> #endif
> diff --git a/drivers/staging/mt7621-mmc/dbg.h b/drivers/staging/mt7621-mmc/dbg.h
> index 2d447b2d92ae..d100324aa3fe 100644
> --- a/drivers/staging/mt7621-mmc/dbg.h
> +++ b/drivers/staging/mt7621-mmc/dbg.h
> @@ -93,7 +93,8 @@ enum msdc_dbg {
>
> extern unsigned int sd_debug_zone[4];
> #define TAG "msdc"
> -void msdc_debug_proc_init(void);
> +void __init msdc_debug_proc_init(void);
> +void __exit msdc_debug_proc_deinit(void);
>
> u32 msdc_time_calc(u32 old_L32, u32 old_H32, u32 new_L32, u32 new_H32);
> void msdc_performance(u32 opcode, u32 sizes, u32 bRx, u32 ticks);
> diff --git a/drivers/staging/mt7621-mmc/sd.c b/drivers/staging/mt7621-mmc/sd.c
> index 9074848a8251..306b3b46f7c9 100644
> --- a/drivers/staging/mt7621-mmc/sd.c
> +++ b/drivers/staging/mt7621-mmc/sd.c
> @@ -1841,6 +1841,10 @@ static int __init mt_msdc_init(void)
>
> static void __exit mt_msdc_exit(void)
> {
> +#if defined(MT6575_SD_DEBUG)
> + msdc_debug_proc_deinit();
> +#endif

You shouldn't need a #ifdef in .c code. In the .h file provide an
"empty" function for this if the option is not defined. Then you can
just call this function in the .c file no matter what.

thanks,

greg k-h