diff mbox

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

Message ID 20090620231151.GA21477@lackof.org (mailing list archive)
State Superseded
Headers show

Commit Message

Grant Grundler June 20, 2009, 11:11 p.m. UTC
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

James Bottomley June 20, 2009, 11:26 p.m. UTC | #1
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


--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Grundler June 20, 2009, 11:29 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Grundler June 23, 2009, 6:31 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Carlos O'Donell June 23, 2009, 11:39 a.m. UTC | #4
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grant Grundler June 24, 2009, 5:31 a.m. UTC | #5
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
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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) {