fix kmalloc() in aha1542.c

Hi,

In drivers/scsi/aha1542.c, kmalloc() is called with no memtype (i.e. without some flag like GFP_KERNEL). The following patch will fix that.

@@ -704,7 +704,7 @@
 #endif
 		int i;
 		ccb[mbo].op = 2;	/* SCSI Initiator Command  w/scatter-gather */
-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
 		cptr = (struct chain *) SCpnt->host_scribble;
 		if (cptr = NULL)

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

|-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
|+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| 		cptr = (struct chain *) SCpnt->host_scribble;

I didn't look to much at your patch but
because you are at it can you please audit that kmalloc too?

tnx

Daniele.




"I could have made money this way, and perhaps amused myself writing code. 
But I knew that at the end of my career, I would look back on years of 
building walls to divide people, and feel I had spent my life making the 
world a worse place."                               
                                                          Richard Stallman

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

GFP_DMA is a helper flag that tells the kernel that the memory should be suitable for DMA purposes. But it doesn't necessarily say what type of memory it should be allocating (i.e. should the memory be allocated on behalf of the user? should the allocation be done atomically? etc).
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

The address returned by kmalloc() seems to be checked three lines later. Though it calls panic() if the address is NULL. I'm not sure if that's the correct way to fail on an allocation.
_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

On Tue, 20 Jan 2004 07:33:10 -0800 Timmy Yee <[email protected]> wrote:

| On Mon, Jan 19, 2004 at 01:37:15PM +0100, Daniele Bellucci wrote:
| > 
| > |-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
| > |+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| > | 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| > | 		cptr = (struct chain *) SCpnt->host_scribble;
| > 
| > I didn't look to much at your patch but 
| > because you are at it can you please audit that kmalloc too?
| 
| The address returned by kmalloc() seems to be checked three lines later.
| Though it calls panic() if the address is NULL. I'm not sure if that's the
| correct way to fail on an allocation.

Preferably not.

--
~Randy
kernel-janitors project: http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list [email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

On Tue, 20 Jan 2004 14:55:55 -0800 "Randy.Dunlap" <[email protected]> wrote:

| On Tue, 20 Jan 2004 07:33:10 -0800 Timmy Yee <[email protected]> wrote:
| 
| | On Mon, Jan 19, 2004 at 01:37:15PM +0100, Daniele Bellucci wrote:
| | > 
| | > |-		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_DMA);
| | > |+		SCpnt->host_scribble = (unsigned char *) kmalloc(512, GFP_KERNEL | GFP_DMA);
| | > | 		sgpnt = (struct scatterlist *) SCpnt->request_buffer;
| | > | 		cptr = (struct chain *) SCpnt->host_scribble;
| | > 
| | > I didn't look to much at your patch but 
| | > because you are at it can you please audit that kmalloc too?
| | 
| | The address returned by kmalloc() seems to be checked three lines later.
| | Though it calls panic() if the address is NULL. I'm not sure if that's the
| | correct way to fail on an allocation.
| 
| Preferably not.

Better answer:
It should just return a non-0 value from the queuecommand() function in that case, so that the SCSI mid-layer will retry the command.

Timmy, care to send that patch?

Thanks,
--
~Randy
kernel-janitors project: http://janitor.kernelnewbies.org/
_______________________________________________
Kernel-janitors mailing list [email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors