Fix gcc 4.4 warning in lba_pci.c

PATCH In review Grant Grundler 4 participants 13 messages Jun 20, 2009 – Jun 24, 2009 PA-RISC architecture

Share & tools

Grant Grundler

author

2009-06-20 22:46 UTC #0 lore raw

PATCH 1/1 Fix gcc 4.4 warning in lba_pci.c

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 <[email protected]>

---

drivers/parisc/lba_pci.c +6 -2

index 59fbbf1..7535cb3 100644
@@ -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);

Grant Grundler

author

2009-06-20 23:11 UTC #1 lore raw

Quoted 7 lines · 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".

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

Signed-off-by Grant Grundler <[email protected]>

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

drivers/parisc/lba_pci.c +19 -9

index 59fbbf1..6585c29 100644
@@ -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) {

James Bottomley

maintainer

2009-06-20 23:26 UTC #2 lore raw

Quoted 11 lines · Grant Grundler

Quoted 7 lines · 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".
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

Grant Grundler

author

2009-06-20 23:29 UTC #3 lore raw

Quoted 10 lines · James Bottomley

Quoted 9 lines · Grant Grundler
Quoted 7 lines · 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".

Quoted 6 lines

Quoted 2 lines
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

John David Anglin

2009-06-21 00:06 UTC #4 lore raw

Quoted 4 lines

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

Signature · 2 lines

J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Grant Grundler

author

2009-06-23 06:31 UTC #5 lore raw

Quoted 7 lines · John David Anglin

Quoted 4 lines
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

Carlos O'Donell

2009-06-23 11:39 UTC #6 lore raw

Quoted 13 lines · Grant Grundler

Quoted 7 lines · John David Anglin
Quoted 4 lines
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:
(quoted diff) +1 -1
-static match_table_t __initconst tokens = {
+static const match_table_t tokens __initconst = {

Quoted 1 line

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.

Grant Grundler

author

2009-06-24 05:31 UTC #7 lore raw

Quoted 1 line · Carlos O'Donell

On Tue, Jun 23, 2009 at 2:31 AM, Grant

...

Quoted 3 lines

Quoted 1 line
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

John David Anglin

2009-06-24 13:47 UTC #8 lore raw

Quoted 10 lines · Carlos O'Donell

Quoted 1 line
On Tue, Jun 23, 2009 at 2:31 AM, Grant
...
Quoted 3 lines
Quoted 1 line
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

Signature · 2 lines

J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Grant Grundler

author

2009-06-20 23:34 UTC #9 lore raw

In reply to James Bottomley

On Sat, Jun 20, 2009 at 06:26:15PM -0500, James Bottomley wrote:
...

Quoted 6 lines

Quoted 2 lines
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 <[email protected]>

----
Again...still not tested. :(

drivers/parisc/lba_pci.c +22 -9

index 59fbbf1..a30e668 100644
@@ -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 */

John David Anglin

2009-06-20 23:39 UTC #10 lore raw

drivers/parisc/lba_pci.c static void · line 994

Quoted 5 lines

drivers/parisc/lba_pci.c +5 -0

+	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

Signature · 2 lines

J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Grant Grundler

author

2009-06-20 23:51 UTC #11 lore raw

Quoted 7 lines · John David Anglin

drivers/parisc/lba_pci.c static void · line 994
Quoted 5 lines
drivers/parisc/lba_pci.c +5 -0
+	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

Grant Grundler

author

2009-06-20 23:55 UTC #12 lore raw

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

In reply to 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 <[email protected]>

---
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.

drivers/parisc/lba_pci.c +22 -9

index 59fbbf1..bc7a82d 100644
@@ -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 */