Re: [PATCH] V2 scsi_mod: Add a new parameter to scsi_mod to control

[Laurence Oberman <loberman@redhat.com>]

On Thu, 2024-04-18 at 14:10 -0400, Laurence Oberman wrote:
> Resend of this patch as V2 against Martin's tree.
> Changes: Removed initialization of global variable
> storage_quiet_discovery
> 
> This new parameter storage_quiet_discovery defaults to 0 and behavior
> is
> unchanged. If its set to 1 on the kernel line then sd_printk and
> sdev_printk are disabled for printing. The default logging can be
> re-enabled any time after boot using /etc/sysctl.conf by setting
> dev.scsi.storage_quiet_discovery = 0.
> systctl -w dev.scsi.storage_quiet_discovery=0 will also change it
> immediately back to logging. i
> Users can leave it set to 1 on the kernel line and 0 in the conf file
> so it changes back to default after rc.sysinit.
> This solves the tough problem of systems with 1000's of
> storage LUNS consuming a system and preventing it from booting due to
> NMI's and timeouts due to udev triggers.
> 
> Signed-off-by: Laurence Oberman <loberman@redhat.com>
> ---
>  Documentation/scsi/scsi-parameters.rst | 16 ++++++++++++++++
>  Makefile                               |  2 +-
>  drivers/scsi/scsi.c                    |  6 ++++++
>  drivers/scsi/scsi_sysctl.c             |  5 +++++
>  drivers/scsi/sd.h                      | 11 ++++++++---
>  include/scsi/scsi_device.h             |  8 ++++++--
>  6 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/scsi/scsi-parameters.rst
> b/Documentation/scsi/scsi-parameters.rst
> index c42c55e1e25e..ec2372db82bd 100644
> --- a/Documentation/scsi/scsi-parameters.rst
> +++ b/Documentation/scsi/scsi-parameters.rst
> @@ -93,6 +93,22 @@ parameters may be changed at runtime by the
> command
>                         S390-tools package, available for download at
>                         
> https://github.com/ibm-s390-linux/s390-tools/blob/master/scripts/scsi
> _logging_level
>  
> +       scsi_mod.storage_quiet_discovery=
> +                       [SCSI] a parameter to control the printing
> from
> +                        sdev_printk and sd_printk for systems with
> large
> +                        amounts of LUNS.
> +                        Defaults to 0 so unchanged behavior.
> +                        If scsi_mod.storage_quiet_discovery=1 is
> added boot line
> +                        then the messages are not printed and can be
> enabled
> +                        after boot via
> +                        echo 0 >
> +                       
> /sys/module/scsi_mod/parameters/storage_quiet_discovery
> +                        Another option is using
> +                        sysctl -w
> dev.scsi.storage_quiet_discovery=0.
> +                        Leaving this set to 0 in /etc/sysctl.conf
> and setting
> +                        it to 1 on the kernel line will help for
> these large
> +                        LUN count configurations and ensure its back
> on after boot.
> +
>         scsi_mod.scan=  [SCSI] sync (default) scans SCSI busses as
> they are
>                         discovered.  async scans them in kernel
> threads,
>                         allowing boot to proceed.  none ignores them,
> expecting
> diff --git a/Makefile b/Makefile
> index 763b6792d3d5..5effa83a32ab 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@
>  VERSION = 6
>  PATCHLEVEL = 9
>  SUBLEVEL = 0
> -EXTRAVERSION = -rc1
> +EXTRAVERSION = -rc1.lobe
>  NAME = Hurr durr I'ma ninja sloth
>  
>  # *DOCUMENTATION*
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 3e0c0381277a..38db68861bbe 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -76,6 +76,9 @@
>   * Definitions and constants.
>   */
>  
> +int storage_quiet_discovery;
> +EXPORT_SYMBOL(storage_quiet_discovery);
> +
>  /*
>   * Note - the initial logging level can be set here to log events at
> boot time.
>   * After the system is up, you may enable logging via the /proc
> interface.
> @@ -979,6 +982,9 @@ MODULE_LICENSE("GPL");
>  
>  module_param(scsi_logging_level, int, S_IRUGO|S_IWUSR);
>  MODULE_PARM_DESC(scsi_logging_level, "a bit mask of logging
> levels");
> +module_param(storage_quiet_discovery, int, S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(storage_quiet_discovery, "If set to 1 will silence
> SCSI and \
> +               ALUA discovery logging on boot");
>  
>  static int __init init_scsi(void)
>  {
> diff --git a/drivers/scsi/scsi_sysctl.c b/drivers/scsi/scsi_sysctl.c
> index 093774d77534..13c7ee4a6212 100644
> --- a/drivers/scsi/scsi_sysctl.c
> +++ b/drivers/scsi/scsi_sysctl.c
> @@ -18,6 +18,11 @@ static struct ctl_table scsi_table[] = {
>           .maxlen       = sizeof(scsi_logging_level),
>           .mode         = 0644,
>           .proc_handler = proc_dointvec },
> +       { .procname     = "storage_quiet_discovery",
> +         .data         = &storage_quiet_discovery,
> +         .maxlen       = sizeof(storage_quiet_discovery),
> +         .mode         = 0644,
> +         .proc_handler = proc_dointvec },
>  };
>  
>  static struct ctl_table_header *scsi_table_header;
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 5c4285a582b2..e1cff37ca69e 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -162,11 +162,16 @@ static inline struct scsi_disk
> *scsi_disk(struct gendisk *disk)
>         return disk->private_data;
>  }
>  
> +extern int storage_quiet_discovery;
> +
>  #define sd_printk(prefix, sdsk, fmt,
> a...)                             \
> -        (sdsk)->disk
> ?                                                 \
> -             sdev_prefix_printk(prefix, (sdsk)-
> >device,                \
> +       do
> {                                                            \
> +               if
> (!storage_quiet_discovery)                           \
> +                (sdsk)->disk
> ?                                         \
> +                       sdev_prefix_printk(prefix, (sdsk)-
> >device,      \
>                                  (sdsk)->disk->disk_name, fmt, ##a)
> :   \
> -             sdev_printk(prefix, (sdsk)->device, fmt, ##a)
> +                       sdev_printk(prefix, (sdsk)->device, fmt,
> ##a);  \
> +       } while (0)
>  
>  #define sd_first_printk(prefix, sdsk, fmt,
> a...)                       \
>         do
> {                                                            \
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 9c540f5468eb..0ad0c387883e 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -302,8 +302,12 @@ __printf(4, 5) void
>  sdev_prefix_printk(const char *, const struct scsi_device *, const
> char *,
>                 const char *, ...);
>  
> -#define sdev_printk(l, sdev, fmt,
> a...)                                \
> -       sdev_prefix_printk(l, sdev, NULL, fmt, ##a)
> +extern int storage_quiet_discovery;
> +
> +#define sdev_printk(l, sdev, fmt, a...) ({                     \
> +       if (!storage_quiet_discovery)                           \
> +               sdev_prefix_printk(l, sdev, NULL, fmt, ##a);    \
> +       })
>  
>  __printf(3, 4) void
>  scmd_printk(const char *, const struct scsi_cmnd *, const char *,
> ...);

Replying to my own message:

After review will send a V3

I just noticed the patch included the Makefile and that should not be
included.
Also Subject was truncated so will fix that too.

Thanks
Laurence