[patch 06/17] scsi: fix section mismatch in aic94xx

[patch 06/17] scsi: fix section mismatch in aic94xx

[akpm]

From: Sam Ravnborg <sam@ravnborg.org>

Fix following warnings:
WARNING: vmlinux.o(.data+0x17aa88): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
WARNING: vmlinux.o(.data+0x17aa98): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
WARNING: vmlinux.o(.data+0x17aaa8): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9405_setup()

asd_pcidev_data is only used by __devinit asd_pci_probe.  So mark is const and
annotate it __devinitconst to fix the warnings.

[akpm@linux-foundation.org: fix warning]
Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Gilbert Wu <gilbert_wu@adaptec.com>
Cc: James Bottomley <James.Bottomley@SteelEye.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/scsi/aic94xx/aic94xx_init.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff -puN drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx drivers/scsi/aic94xx/aic94xx_init.c
--- a/drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx
+++ a/drivers/scsi/aic94xx/aic94xx_init.c
@@ -529,10 +529,10 @@ static void asd_remove_dev_attrs(struct 
 /* The first entry, 0, is used for dynamic ids, the rest for devices
  * we know about.
  */
-static struct asd_pcidev_struct {
+static const struct asd_pcidev_struct {
 	const char * name;
 	int (*setup)(struct asd_ha_struct *asd_ha);
-} asd_pcidev_data[] = {
+} asd_pcidev_data[] __devinitconst = {
 	/* Id 0 is used for dynamic ids. */
 	{ .name  = "Adaptec AIC-94xx SAS/SATA Host Adapter",
 	  .setup = asd_aic9410_setup
@@ -735,7 +735,7 @@ static int asd_unregister_sas_ha(struct 
 static int __devinit asd_pci_probe(struct pci_dev *dev,
 				   const struct pci_device_id *id)
 {
-	struct asd_pcidev_struct *asd_dev;
+	const struct asd_pcidev_struct *asd_dev;
 	unsigned asd_id = (unsigned) id->driver_data;
 	struct asd_ha_struct *asd_ha;
 	struct Scsi_Host *shost;
_

Re: [patch 06/17] scsi: fix section mismatch in aic94xx

[James Bottomley]

On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> From: Sam Ravnborg <sam@ravnborg.org>
> 
> Fix following warnings:
> WARNING: vmlinux.o(.data+0x17aa88): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> WARNING: vmlinux.o(.data+0x17aa98): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> WARNING: vmlinux.o(.data+0x17aaa8): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9405_setup()
> 
> asd_pcidev_data is only used by __devinit asd_pci_probe.  So mark is const and
> annotate it __devinitconst to fix the warnings.
> 
> [akpm@linux-foundation.org: fix warning]
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Gilbert Wu <gilbert_wu@adaptec.com>
> Cc: James Bottomley <James.Bottomley@SteelEye.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/scsi/aic94xx/aic94xx_init.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff -puN drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx drivers/scsi/aic94xx/aic94xx_init.c
> --- a/drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx
> +++ a/drivers/scsi/aic94xx/aic94xx_init.c
> @@ -529,10 +529,10 @@ static void asd_remove_dev_attrs(struct 
>  /* The first entry, 0, is used for dynamic ids, the rest for devices
>   * we know about.
>   */
> -static struct asd_pcidev_struct {
> +static const struct asd_pcidev_struct {
>  	const char * name;
>  	int (*setup)(struct asd_ha_struct *asd_ha);
> -} asd_pcidev_data[] = {
> +} asd_pcidev_data[] __devinitconst = {
>  	/* Id 0 is used for dynamic ids. */
>  	{ .name  = "Adaptec AIC-94xx SAS/SATA Host Adapter",
>  	  .setup = asd_aic9410_setup

If your tools say this patch is correct, then the tools are wrong.

This patch actually takes something that's functionally correct but your
tools can't see it and transforms it to something that's actually
functionally wrong ... and the tool should spot that.

the problem is the contents of the .name field ... they can't live in
the devinit rodata section because we actually take a pointer to it (as
asd_ha->name and later ha->sas_ha_name) in asd_pci_probe() that lives
the entire lifetime of the driver and is used.  derefing it will trigger
a bug after the __devinit section is discarded in the non hotplug case.

James

Re: [patch 06/17] scsi: fix section mismatch in aic94xx

[Sam Ravnborg]

On Sun, Mar 30, 2008 at 01:05:04PM -0500, James Bottomley wrote:
> On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> > From: Sam Ravnborg <sam@ravnborg.org>
> > 
> > Fix following warnings:
> > WARNING: vmlinux.o(.data+0x17aa88): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> > WARNING: vmlinux.o(.data+0x17aa98): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> > WARNING: vmlinux.o(.data+0x17aaa8): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9405_setup()
> > 
> > asd_pcidev_data is only used by __devinit asd_pci_probe.  So mark is const and
> > annotate it __devinitconst to fix the warnings.
> > 
> > [akpm@linux-foundation.org: fix warning]
> > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Gilbert Wu <gilbert_wu@adaptec.com>
> > Cc: James Bottomley <James.Bottomley@SteelEye.com>
> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > ---
> > 
> >  drivers/scsi/aic94xx/aic94xx_init.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff -puN drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx drivers/scsi/aic94xx/aic94xx_init.c
> > --- a/drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx
> > +++ a/drivers/scsi/aic94xx/aic94xx_init.c
> > @@ -529,10 +529,10 @@ static void asd_remove_dev_attrs(struct 
> >  /* The first entry, 0, is used for dynamic ids, the rest for devices
> >   * we know about.
> >   */
> > -static struct asd_pcidev_struct {
> > +static const struct asd_pcidev_struct {
> >  	const char * name;
> >  	int (*setup)(struct asd_ha_struct *asd_ha);
> > -} asd_pcidev_data[] = {
> > +} asd_pcidev_data[] __devinitconst = {
> >  	/* Id 0 is used for dynamic ids. */
> >  	{ .name  = "Adaptec AIC-94xx SAS/SATA Host Adapter",
> >  	  .setup = asd_aic9410_setup
> 
> If your tools say this patch is correct, then the tools are wrong.
> 
> This patch actually takes something that's functionally correct but your
> tools can't see it and transforms it to something that's actually
> functionally wrong ... and the tool should spot that.
> 
> the problem is the contents of the .name field ... they can't live in
> the devinit rodata section because we actually take a pointer to it (as
> asd_ha->name and later ha->sas_ha_name) in asd_pci_probe() that lives
> the entire lifetime of the driver and is used.  derefing it will trigger
> a bug after the __devinit section is discarded in the non hotplug case.
asd_pcidev_dada[].name is dereferenced only in the above mentioned function.
The string is used in other places - correct. But this patch does
not try to move the defined string anywhere only the data member
holding the pointer to the string.

Take a look at the content of .rodata.str1.1 to see the string
being contained there.

	Sam

Re: [patch 06/17] scsi: fix section mismatch in aic94xx

[James Bottomley]

On Sun, 2008-03-30 at 20:44 +0200, Sam Ravnborg wrote: 
> On Sun, Mar 30, 2008 at 01:05:04PM -0500, James Bottomley wrote:
> > On Fri, 2008-03-28 at 14:48 -0700, akpm@linux-foundation.org wrote:
> > > From: Sam Ravnborg <sam@ravnborg.org>
> > > 
> > > Fix following warnings:
> > > WARNING: vmlinux.o(.data+0x17aa88): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> > > WARNING: vmlinux.o(.data+0x17aa98): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9410_setup()
> > > WARNING: vmlinux.o(.data+0x17aaa8): Section mismatch in reference from the variable asd_pcidev_data to the function .devinit.text:asd_aic9405_setup()
> > > 
> > > asd_pcidev_data is only used by __devinit asd_pci_probe.  So mark is const and
> > > annotate it __devinitconst to fix the warnings.
> > > 
> > > [akpm@linux-foundation.org: fix warning]
> > > Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> > > Cc: Gilbert Wu <gilbert_wu@adaptec.com>
> > > Cc: James Bottomley <James.Bottomley@SteelEye.com>
> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> > > ---
> > > 
> > >  drivers/scsi/aic94xx/aic94xx_init.c |    6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff -puN drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx drivers/scsi/aic94xx/aic94xx_init.c
> > > --- a/drivers/scsi/aic94xx/aic94xx_init.c~scsi-fix-section-mismatch-in-aic94xx
> > > +++ a/drivers/scsi/aic94xx/aic94xx_init.c
> > > @@ -529,10 +529,10 @@ static void asd_remove_dev_attrs(struct 
> > >  /* The first entry, 0, is used for dynamic ids, the rest for devices
> > >   * we know about.
> > >   */
> > > -static struct asd_pcidev_struct {
> > > +static const struct asd_pcidev_struct {
> > >  	const char * name;
> > >  	int (*setup)(struct asd_ha_struct *asd_ha);
> > > -} asd_pcidev_data[] = {
> > > +} asd_pcidev_data[] __devinitconst = {
> > >  	/* Id 0 is used for dynamic ids. */
> > >  	{ .name  = "Adaptec AIC-94xx SAS/SATA Host Adapter",
> > >  	  .setup = asd_aic9410_setup
> > 
> > If your tools say this patch is correct, then the tools are wrong.
> > 
> > This patch actually takes something that's functionally correct but your
> > tools can't see it and transforms it to something that's actually
> > functionally wrong ... and the tool should spot that.
> > 
> > the problem is the contents of the .name field ... they can't live in
> > the devinit rodata section because we actually take a pointer to it (as
> > asd_ha->name and later ha->sas_ha_name) in asd_pci_probe() that lives
> > the entire lifetime of the driver and is used.  derefing it will trigger
> > a bug after the __devinit section is discarded in the non hotplug case.
> asd_pcidev_dada[].name is dereferenced only in the above mentioned function.
> The string is used in other places - correct. But this patch does
> not try to move the defined string anywhere only the data member
> holding the pointer to the string.
> 
> Take a look at the content of .rodata.str1.1 to see the string
> being contained there.

So none of the strings we use in init sections are actually placed into
init data?  I had thought that was one of the things that got fixed in
the quest for page savings, especially as we have quite a few of
them ...

In that case this is OK ... however, assuming you're planning to fix the
non discarded init strings problem, won't this fix become wrong at that
point?

James