[PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

gcc 4.4 warns about:
drivers/parisc/lba_pci.c: In function 'lba_pat_resources':
drivers/parisc/lba_pci.c:1099: warning: the frame size of 8280 bytes is larger than 4096 bytes

The problem is we declare two large structures on the stack. They don't need
to be on the stack since they are only used during LBA initialization (which
is serialized). Moving to be "static".

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
---

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 59fbbf1..7535cb3 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -980,13 +980,17 @@ static void
 lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 {
 	unsigned long bytecnt;
-	pdc_pat_cell_mod_maddr_block_t pa_pdc_cell;	/* PA_VIEW */
-	pdc_pat_cell_mod_maddr_block_t io_pdc_cell;	/* IO_VIEW */
 	long io_count;
 	long status;	/* PDC return status */
 	long pa_count;
 	int i;
 
+	/* We don't need additional locking around the use of pdc_pat_cell
+	**  since init time PDC device discovery is already serialized.
+	*/
+	static pdc_pat_cell_mod_maddr_block_t pa_pdc_cell;	/* PA_VIEW */
+	static pdc_pat_cell_mod_maddr_block_t io_pdc_cell;	/* IO_VIEW */
+
 	/* return cell module (IO view) */
 	status = pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
 				PA_VIEW, & pa_pdc_cell);

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 04:46:13PM -0600, Grant Grundler wrote:
> gcc 4.4 warns about:
> drivers/parisc/lba_pci.c: In function 'lba_pat_resources':
> drivers/parisc/lba_pci.c:1099: warning: the frame size of 8280 bytes is larger than 4096 bytes
> 
> The problem is we declare two large structures on the stack. They don't need
> to be on the stack since they are only used during LBA initialization (which
> is serialized). Moving to be "static".

Take 2. Per Kyle's request (offlist), use kzalloc instead since it's not
ever used again after boot.

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>

----
Not tested! Need to fix other b0rkage before I can test.
And feel free to use either version of this patch. Both are trivial.

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 59fbbf1..6585c29 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -980,28 +980,38 @@ static void
 lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 {
 	unsigned long bytecnt;
-	pdc_pat_cell_mod_maddr_block_t pa_pdc_cell;	/* PA_VIEW */
-	pdc_pat_cell_mod_maddr_block_t io_pdc_cell;	/* IO_VIEW */
 	long io_count;
 	long status;	/* PDC return status */
 	long pa_count;
+	pdc_pat_cell_mod_maddr_block_t *pa_pdc_cell;	/* PA_VIEW */
+	pdc_pat_cell_mod_maddr_block_t *io_pdc_cell;	/* IO_VIEW */
 	int i;
 
+	pa_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
+	if (!pa_pdc_cell)
+		return;
+
+	io_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
+	if (!pa_pdc_cell) {
+		kfree(pa_pdc_cell);
+		return;
+	}
+
 	/* return cell module (IO view) */
 	status = pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				PA_VIEW, & pa_pdc_cell);
-	pa_count = pa_pdc_cell.mod[1];
+				PA_VIEW, pa_pdc_cell);
+	pa_count = pa_pdc_cell->mod[1];
 
 	status |= pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				IO_VIEW, &io_pdc_cell);
-	io_count = io_pdc_cell.mod[1];
+				IO_VIEW, io_pdc_cell);
+	io_count = io_pdc_cell->mod[1];
 
 	/* We've already done this once for device discovery...*/
 	if (status != PDC_OK) {
 		panic("pdc_pat_cell_module() call failed for LBA!\n");
 	}
 
-	if (PAT_GET_ENTITY(pa_pdc_cell.mod_info) != PAT_ENTITY_LBA) {
+	if (PAT_GET_ENTITY(pa_pdc_cell->mod_info) != PAT_ENTITY_LBA) {
 		panic("pdc_pat_cell_module() entity returned != PAT_ENTITY_LBA!\n");
 	}
 
@@ -1016,8 +1026,8 @@ lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 		} *p, *io;
 		struct resource *r;
 
-		p = (void *) &(pa_pdc_cell.mod[2+i*3]);
-		io = (void *) &(io_pdc_cell.mod[2+i*3]);
+		p = (void *) &(pa_pdc_cell->mod[2+i*3]);
+		io = (void *) &(io_pdc_cell->mod[2+i*3]);
 
 		/* Convert the PAT range data to PCI "struct resource" */
 		switch(p->type & 0xff) {

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[James Bottomley]

On Sat, 2009-06-20 at 17:11 -0600, Grant Grundler wrote:
> On Sat, Jun 20, 2009 at 04:46:13PM -0600, Grant Grundler wrote:
> > gcc 4.4 warns about:
> > drivers/parisc/lba_pci.c: In function 'lba_pat_resources':
> > drivers/parisc/lba_pci.c:1099: warning: the frame size of 8280 bytes is larger than 4096 bytes
> > 
> > The problem is we declare two large structures on the stack. They don't need
> > to be on the stack since they are only used during LBA initialization (which
> > is serialized). Moving to be "static".
> 
> Take 2. Per Kyle's request (offlist), use kzalloc instead since it's not
> ever used again after boot.

Um, wouldn't one of the points of using kzalloc over a static allocation
be to free the memory again after we've finished using it?  Otherwise we
leek a page for every lba.

James

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 06:26:15PM -0500, James Bottomley wrote:
> On Sat, 2009-06-20 at 17:11 -0600, Grant Grundler wrote:
> > On Sat, Jun 20, 2009 at 04:46:13PM -0600, Grant Grundler wrote:
> > > gcc 4.4 warns about:
> > > drivers/parisc/lba_pci.c: In function 'lba_pat_resources':
> > > drivers/parisc/lba_pci.c:1099: warning: the frame size of 8280 bytes is larger than 4096 bytes
> > > 
> > > The problem is we declare two large structures on the stack. They don't need
> > > to be on the stack since they are only used during LBA initialization (which
> > > is serialized). Moving to be "static".
> > 
> > Take 2. Per Kyle's request (offlist), use kzalloc instead since it's not
> > ever used again after boot.
> 
> Um, wouldn't one of the points of using kzalloc over a static allocation
> be to free the memory again after we've finished using it?  Otherwise we
> leek a page for every lba.

Doh...of course!
I was too worried about the error case to think about the regular release.
I'll repost a clean version.

In the meantime...if someone can explain this error I could build and test:
fs/nfs/nfsroot.c:400: error: __setup_str_nfs_root_setup causes a section type conflict

something to do with __setup(nfs_root_setup) but it's not obvious to me what.


thanks,
grant

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[John David Anglin]

> In the meantime...if someone can explain this error I could build and test:
> fs/nfs/nfsroot.c:400: error: __setup_str_nfs_root_setup causes a section type conflict
> 
> something to do with __setup(nfs_root_setup) but it's not obvious to me what.

Google says see:
http://lkml.org/lkml/2009/5/26/329

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 08:06:34PM -0400, John David Anglin wrote:
> > In the meantime...if someone can explain this error I could build and test:
> > fs/nfs/nfsroot.c:400: error: __setup_str_nfs_root_setup causes a section type conflict
> > 
> > something to do with __setup(nfs_root_setup) but it's not obvious to me what.
> 
> Google says see:
> http://lkml.org/lkml/2009/5/26/329

Thank you...removing the __initconst added by this change:
-static match_table_t __initconst tokens = {
+static const match_table_t tokens __initconst = {

allows me to build. Is no one else building with NFSROOT enabled?

thanks,
grant

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Carlos O'Donell]

On Tue, Jun 23, 2009 at 2:31 AM, Grant
Grundler<grundler@parisc-linux.org> wrote:
> On Sat, Jun 20, 2009 at 08:06:34PM -0400, John David Anglin wrote:
>> > In the meantime...if someone can explain this error I could build and test:
>> > fs/nfs/nfsroot.c:400: error: __setup_str_nfs_root_setup causes a section type conflict
>> >
>> > something to do with __setup(nfs_root_setup) but it's not obvious to me what.
>>
>> Google says see:
>> http://lkml.org/lkml/2009/5/26/329
>
> Thank you...removing the __initconst added by this change:
> -static match_table_t __initconst tokens = {
> +static const match_table_t tokens __initconst = {
>
> allows me to build. Is no one else building with NFSROOT enabled?

I boot directly from disk on both my test machines (a500, c3k).

Cheers,
Carlos.

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Tue, Jun 23, 2009 at 07:39:37AM -0400, Carlos O'Donell wrote:
> On Tue, Jun 23, 2009 at 2:31 AM, Grant
...
> > allows me to build. Is no one else building with NFSROOT enabled?
> 
> I boot directly from disk on both my test machines (a500, c3k).

Me too in general...but I still have that enabled since the
Cupertino test ring have multiple machines and it's quite easy
to test with NFS root.

thanks,
grant

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[John David Anglin]

> On Tue, Jun 23, 2009 at 07:39:37AM -0400, Carlos O'Donell wrote:
> > On Tue, Jun 23, 2009 at 2:31 AM, Grant
> ...
> > > allows me to build. Is no one else building with NFSROOT enabled?
> > 
> > I boot directly from disk on both my test machines (a500, c3k).
> 
> Me too in general...but I still have that enabled since the
> Cupertino test ring have multiple machines and it's quite easy
> to test with NFS root.

I'm using this for embedded development (no hard drive).

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 06:26:15PM -0500, James Bottomley wrote:
...
> > Take 2. Per Kyle's request (offlist), use kzalloc instead since it's not
> > ever used again after boot.
> 
> Um, wouldn't one of the points of using kzalloc over a static allocation
> be to free the memory again after we've finished using it?  Otherwise we
> leek a page for every lba.

James,
Thanks again for gating my utter fail.
Fixed in this version.

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
----
Again...still not tested. :(

diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 59fbbf1..a30e668 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -980,28 +980,38 @@ static void
 lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 {
 	unsigned long bytecnt;
-	pdc_pat_cell_mod_maddr_block_t pa_pdc_cell;	/* PA_VIEW */
-	pdc_pat_cell_mod_maddr_block_t io_pdc_cell;	/* IO_VIEW */
 	long io_count;
 	long status;	/* PDC return status */
 	long pa_count;
+	pdc_pat_cell_mod_maddr_block_t *pa_pdc_cell;	/* PA_VIEW */
+	pdc_pat_cell_mod_maddr_block_t *io_pdc_cell;	/* IO_VIEW */
 	int i;
 
+	pa_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
+	if (!pa_pdc_cell)
+		return;
+
+	io_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
+	if (!pa_pdc_cell) {
+		kfree(pa_pdc_cell);
+		return;
+	}
+
 	/* return cell module (IO view) */
 	status = pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				PA_VIEW, & pa_pdc_cell);
-	pa_count = pa_pdc_cell.mod[1];
+				PA_VIEW, pa_pdc_cell);
+	pa_count = pa_pdc_cell->mod[1];
 
 	status |= pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				IO_VIEW, &io_pdc_cell);
-	io_count = io_pdc_cell.mod[1];
+				IO_VIEW, io_pdc_cell);
+	io_count = io_pdc_cell->mod[1];
 
 	/* We've already done this once for device discovery...*/
 	if (status != PDC_OK) {
 		panic("pdc_pat_cell_module() call failed for LBA!\n");
 	}
 
-	if (PAT_GET_ENTITY(pa_pdc_cell.mod_info) != PAT_ENTITY_LBA) {
+	if (PAT_GET_ENTITY(pa_pdc_cell->mod_info) != PAT_ENTITY_LBA) {
 		panic("pdc_pat_cell_module() entity returned != PAT_ENTITY_LBA!\n");
 	}
 
@@ -1016,8 +1026,8 @@ lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 		} *p, *io;
 		struct resource *r;
 
-		p = (void *) &(pa_pdc_cell.mod[2+i*3]);
-		io = (void *) &(io_pdc_cell.mod[2+i*3]);
+		p = (void *) &(pa_pdc_cell->mod[2+i*3]);
+		io = (void *) &(io_pdc_cell->mod[2+i*3]);
 
 		/* Convert the PAT range data to PCI "struct resource" */
 		switch(p->type & 0xff) {
@@ -1096,6 +1106,9 @@ lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 			break;
 		}
 	}
+
+	kfree(pa_pdc_cell);
+	kfree(io_pdc_cell);
 }
 #else
 /* keep compiler from complaining about missing declarations */

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[John David Anglin]

> +	pa_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
> +	if (!pa_pdc_cell)
> +		return;
> +
> +	io_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));

The kzalloc calls are missing an argument...

Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Re: [PATCH][linux-2.6] Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 07:39:28PM -0400, John David Anglin wrote:
> > +	pa_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
> > +	if (!pa_pdc_cell)
> > +		return;
> > +
> > +	io_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t));
> 
> The kzalloc calls are missing an argument...

Doh #2....thank you, Dave. Time for a nap. 5h is clearly not enough.
I'll risk posting another version even though I still can't build.

BTW, here is the offending error:
fs/nfs/nfsroot.c:400: error: __setup_str_nfs_root_setup causes a section type conflict

and the offending line 400 after preprocessing:
static const char __setup_str_nfs_root_setup[] __attribute__ ((__section__(".init.rodata"))) __attribute__((aligned(1))) = "nfsroot="; static struct obs_kernel_param __setup_nfs_root_setup __attribute__((__used__)) __attribute__ ((__section__(".init.setup"))) __attribute__((aligned((sizeof(long))))) = { __setup_str_nfs_root_setup, nfs_root_setup, 0 };

thanks,
grant

Re: [PATCH][linux-2.6] v4 : Fix gcc 4.4 warning in lba_pci.c

[Grant Grundler]

On Sat, Jun 20, 2009 at 04:46:13PM -0600, Grant Grundler wrote:
gcc 4.4 warns about:
drivers/parisc/lba_pci.c: In function 'lba_pat_resources':
drivers/parisc/lba_pci.c:1099: warning: the frame size of 8280 bytes is larger than 4096 bytes

The problem is we declare two large structures on the stack. They don't need
to be on the stack since they are only used during LBA initialization (which
is serialized). Moving to be "static".

Signed-off-by: Grant Grundler <grundler@parisc-linux.org>
---
Take #3. Still can not build a kernel to test. Thanks to James Bottomley and
John David Anglin for catching the dumbest errors I've done in a long time.


diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index 59fbbf1..bc7a82d 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -980,28 +980,38 @@ static void
 lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 {
 	unsigned long bytecnt;
-	pdc_pat_cell_mod_maddr_block_t pa_pdc_cell;	/* PA_VIEW */
-	pdc_pat_cell_mod_maddr_block_t io_pdc_cell;	/* IO_VIEW */
 	long io_count;
 	long status;	/* PDC return status */
 	long pa_count;
+	pdc_pat_cell_mod_maddr_block_t *pa_pdc_cell;	/* PA_VIEW */
+	pdc_pat_cell_mod_maddr_block_t *io_pdc_cell;	/* IO_VIEW */
 	int i;
 
+	pa_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t), GFP_KERNEL);
+	if (!pa_pdc_cell)
+		return;
+
+	io_pdc_cell = kzalloc(sizeof(pdc_pat_cell_mod_maddr_block_t), GFP_KERNEL);
+	if (!pa_pdc_cell) {
+		kfree(pa_pdc_cell);
+		return;
+	}
+
 	/* return cell module (IO view) */
 	status = pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				PA_VIEW, & pa_pdc_cell);
-	pa_count = pa_pdc_cell.mod[1];
+				PA_VIEW, pa_pdc_cell);
+	pa_count = pa_pdc_cell->mod[1];
 
 	status |= pdc_pat_cell_module(&bytecnt, pa_dev->pcell_loc, pa_dev->mod_index,
-				IO_VIEW, &io_pdc_cell);
-	io_count = io_pdc_cell.mod[1];
+				IO_VIEW, io_pdc_cell);
+	io_count = io_pdc_cell->mod[1];
 
 	/* We've already done this once for device discovery...*/
 	if (status != PDC_OK) {
 		panic("pdc_pat_cell_module() call failed for LBA!\n");
 	}
 
-	if (PAT_GET_ENTITY(pa_pdc_cell.mod_info) != PAT_ENTITY_LBA) {
+	if (PAT_GET_ENTITY(pa_pdc_cell->mod_info) != PAT_ENTITY_LBA) {
 		panic("pdc_pat_cell_module() entity returned != PAT_ENTITY_LBA!\n");
 	}
 
@@ -1016,8 +1026,8 @@ lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 		} *p, *io;
 		struct resource *r;
 
-		p = (void *) &(pa_pdc_cell.mod[2+i*3]);
-		io = (void *) &(io_pdc_cell.mod[2+i*3]);
+		p = (void *) &(pa_pdc_cell->mod[2+i*3]);
+		io = (void *) &(io_pdc_cell->mod[2+i*3]);
 
 		/* Convert the PAT range data to PCI "struct resource" */
 		switch(p->type & 0xff) {
@@ -1096,6 +1106,9 @@ lba_pat_resources(struct parisc_device *pa_dev, struct lba_device *lba_dev)
 			break;
 		}
 	}
+
+	kfree(pa_pdc_cell);
+	kfree(io_pdc_cell);
 }
 #else
 /* keep compiler from complaining about missing declarations */