Re: [PATCH v3 1/6] scsi: Define MODULE_DEVICE_TABLE only if necessary

Re: [PATCH v3 1/6] scsi: Define MODULE_DEVICE_TABLE only if necessary

[James Bottomley]

On Tue, 2025-05-27 at 11:07 +0200, Alexey Gladkov wrote:
> Define MODULE_DEVICE_TABLE only if a structure is defined for it.
> 
> drivers/scsi/BusLogic.c:3735:26: error: use of undeclared identifier
> 'blogic_pci_tbl'
>  3735 | MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);

Well, a) need to cc the scsi list and b) how is this possible when
MODULE_DEVICE_TABLE() has an empty definition if MODULE isn't defined
(so the guard you move should be over an empty statement)?

Regards,

James

Re: [PATCH v3 1/6] scsi: Define MODULE_DEVICE_TABLE only if necessary

[Alexey Gladkov]

On Tue, May 27, 2025 at 07:28:59AM -0400, James Bottomley wrote:
> On Tue, 2025-05-27 at 11:07 +0200, Alexey Gladkov wrote:
> > Define MODULE_DEVICE_TABLE only if a structure is defined for it.
> > 
> > drivers/scsi/BusLogic.c:3735:26: error: use of undeclared identifier
> > 'blogic_pci_tbl'
> >  3735 | MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
> 
> Well, a) need to cc the scsi list

Sorry. I miss it.

> and b) how is this possible when
> MODULE_DEVICE_TABLE() has an empty definition if MODULE isn't defined
> (so the guard you move should be over an empty statement)?

In the next patch:

[PATCH v3 4/6] modpost: Create modalias for builtin modules

I remove this condition for the MODULE_DEVICE_TABLE macro and it will be
always defined.

I put the drivers/scsi/BusLogic.c change before these changes to avoid
errors. Besides, even an empty macro uses a structure name that is not
defined (if MODULE isn't defined). This seems wrong in any case.

-- 
Rgrds, legion

Re: [PATCH v3 1/6] scsi: Define MODULE_DEVICE_TABLE only if necessary

[James Bottomley]

On Tue, 2025-05-27 at 13:54 +0200, Alexey Gladkov wrote:
> On Tue, May 27, 2025 at 07:28:59AM -0400, James Bottomley wrote:
> > On Tue, 2025-05-27 at 11:07 +0200, Alexey Gladkov wrote:
> > > Define MODULE_DEVICE_TABLE only if a structure is defined for it.
> > > 
> > > drivers/scsi/BusLogic.c:3735:26: error: use of undeclared
> > > identifier
> > > 'blogic_pci_tbl'
> > >  3735 | MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
> > 
> > Well, a) need to cc the scsi list
> 
> Sorry. I miss it.
> 
> > and b) how is this possible when MODULE_DEVICE_TABLE() has an empty
> > definition if MODULE isn't defined (so the guard you move should be
> > over an empty statement)?
> 
> In the next patch:
> 
> [PATCH v3 4/6] modpost: Create modalias for builtin modules
> 
> I remove this condition for the MODULE_DEVICE_TABLE macro and it will
> be always defined.

Well, why?  If there's a reason for the table to always exist, wouldn't
the best fix then be to remove the module guards from the PCI table in
the buslogic ... they only really exist to prevent a defined but not
used error which it sounds like you're getting rid of?

Regards,

James

Re: [PATCH v3 1/6] scsi: Define MODULE_DEVICE_TABLE only if necessary

[Alexey Gladkov]

On Tue, May 27, 2025 at 07:58:27AM -0400, James Bottomley wrote:
> On Tue, 2025-05-27 at 13:54 +0200, Alexey Gladkov wrote:
> > On Tue, May 27, 2025 at 07:28:59AM -0400, James Bottomley wrote:
> > > On Tue, 2025-05-27 at 11:07 +0200, Alexey Gladkov wrote:
> > > > Define MODULE_DEVICE_TABLE only if a structure is defined for it.
> > > > 
> > > > drivers/scsi/BusLogic.c:3735:26: error: use of undeclared
> > > > identifier
> > > > 'blogic_pci_tbl'
> > > >  3735 | MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
> > > 
> > > Well, a) need to cc the scsi list
> > 
> > Sorry. I miss it.
> > 
> > > and b) how is this possible when MODULE_DEVICE_TABLE() has an empty
> > > definition if MODULE isn't defined (so the guard you move should be
> > > over an empty statement)?
> > 
> > In the next patch:
> > 
> > [PATCH v3 4/6] modpost: Create modalias for builtin modules
> > 
> > I remove this condition for the MODULE_DEVICE_TABLE macro and it will
> > be always defined.
> 
> Well, why?  If there's a reason for the table to always exist, wouldn't
> the best fix then be to remove the module guards from the PCI table in
> the buslogic ... they only really exist to prevent a defined but not
> used error which it sounds like you're getting rid of?

I wanted to keep the original logic and remove the build error. Before my
changes blogic_pci_tbl was only used when the module was built separately
(MODULE case).

But yes, you are right. In this case, it would be more appropriate to
remove the MODULE condition at all since MODULE_DEVICE_TABLE always
makes sense after my changes.

-- 
Rgrds, legion

[PATCH v4 1/6] scsi: Always define blogic_pci_tbl structure

[Alexey Gladkov]

The blogic_pci_tbl structure is used by the MODULE_DEVICE_TABLE macro.
There is no longer a need to protect it with the MODULE condition, since
this no longer causes the compiler to warn about an unused variable.

Cc: Khalid Aziz <khalid@gonehiking.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/scsi/BusLogic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 1f100270cd38..08e12a3d6703 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
 
 __setup("BusLogic=", blogic_setup);
 
-#ifdef MODULE
 /*static const struct pci_device_id blogic_pci_tbl[] = {
 	{ PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -3731,7 +3730,6 @@ static const struct pci_device_id blogic_pci_tbl[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_FLASHPOINT)},
 	{0, },
 };
-#endif
 MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
 
 module_init(blogic_init);
-- 
2.49.0

[PATCH v3 7/6] scsi: Always define MODULE_DEVICE_TABLE

[Alexey Gladkov]

Since MODULE_DEVICE_TABLE no longer depends on whether the module is
built separately or compiled into the kernel, it now makes sense to
always define DEVICE_TABLE. In this case, even if the module is in the
kernel, correct module.builtin.modaliases will be generated.

Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/scsi/BusLogic.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 8ce2ac9293a3..08e12a3d6703 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
 
 __setup("BusLogic=", blogic_setup);
 
-#ifdef MODULE
 /*static const struct pci_device_id blogic_pci_tbl[] = {
 	{ PCI_VENDOR_ID_BUSLOGIC, PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
@@ -3732,7 +3731,6 @@ static const struct pci_device_id blogic_pci_tbl[] = {
 	{0, },
 };
 MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
-#endif
 
 module_init(blogic_init);
 module_exit(blogic_exit);
-- 
2.49.0

Re: [PATCH v3 7/6] scsi: Always define MODULE_DEVICE_TABLE

[James Bottomley]

On Tue, 2025-05-27 at 15:15 +0200, Alexey Gladkov wrote:
> Since MODULE_DEVICE_TABLE no longer depends on whether the module is
> built separately or compiled into the kernel, it now makes sense to
> always define DEVICE_TABLE. In this case, even if the module is in
> the
> kernel, correct module.builtin.modaliases will be generated.
> 
> Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
>  drivers/scsi/BusLogic.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index 8ce2ac9293a3..08e12a3d6703 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
>  
>  __setup("BusLogic=", blogic_setup);
>  
> -#ifdef MODULE
>  /*static const struct pci_device_id blogic_pci_tbl[] = {
>  	{ PCI_VENDOR_ID_BUSLOGIC,
> PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
>  	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> @@ -3732,7 +3731,6 @@ static const struct pci_device_id
> blogic_pci_tbl[] = {
>  	{0, },
>  };
>  MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
> -#endif

You don't need to do this in two steps.  The original problem of
defined but not used table stopped being a problem when the structure
was converted to static const over ten years ago (the compiler doesn't
warn about unused static consts).

Regards,

James

Re: [PATCH v3 7/6] scsi: Always define MODULE_DEVICE_TABLE

[Alexey Gladkov]

On Tue, May 27, 2025 at 09:22:09AM -0400, James Bottomley wrote:
> On Tue, 2025-05-27 at 15:15 +0200, Alexey Gladkov wrote:
> > Since MODULE_DEVICE_TABLE no longer depends on whether the module is
> > built separately or compiled into the kernel, it now makes sense to
> > always define DEVICE_TABLE. In this case, even if the module is in
> > the
> > kernel, correct module.builtin.modaliases will be generated.
> > 
> > Suggested-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >  drivers/scsi/BusLogic.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> > index 8ce2ac9293a3..08e12a3d6703 100644
> > --- a/drivers/scsi/BusLogic.c
> > +++ b/drivers/scsi/BusLogic.c
> > @@ -3715,7 +3715,6 @@ static void __exit blogic_exit(void)
> >  
> >  __setup("BusLogic=", blogic_setup);
> >  
> > -#ifdef MODULE
> >  /*static const struct pci_device_id blogic_pci_tbl[] = {
> >  	{ PCI_VENDOR_ID_BUSLOGIC,
> > PCI_DEVICE_ID_BUSLOGIC_MULTIMASTER,
> >  	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> > @@ -3732,7 +3731,6 @@ static const struct pci_device_id
> > blogic_pci_tbl[] = {
> >  	{0, },
> >  };
> >  MODULE_DEVICE_TABLE(pci, blogic_pci_tbl);
> > -#endif
> 
> You don't need to do this in two steps.  The original problem of
> defined but not used table stopped being a problem when the structure
> was converted to static const over ten years ago (the compiler doesn't
> warn about unused static consts).

Ah. Ok, I will recreate this patch shortly.

Basically my original plan was to fix compilation errors as a first step,
and second step make MODULE_DEVICE_TABLE be used independently of MODULE.
Because there are a bunch of modules that also use MODULE_DEVICE_TABLE
only if MODULE is defined.

-- 
Rgrds, legion