Re: [PATCH] New QStor SATA/RAID Driver for 2.6.9-rc2
From: Alan Cox
Date: Tue Sep 14 2004 - 12:23:52 EST
On Maw, 2004-09-14 at 16:42, Mark Lord wrote:
> Please email me any errors or corrections you may deem necessary
> for kernel inclusion.
Nothing says its GPL
qstor_printk uses a fixed length buffer yet we have vprintk. It also
appears to be full of magic maybe formatting stuff to optionally insert
things like DRIVER_NAME. Is there a reason for not just using printk ?
qstore_test_logbuf returns the flags value across functions. Not all
platforms can support this (see Rusty's unreliable guide)
What is qstor_alloc about ?
What happens if qstor_scsi_proc_write is passed a length of
0xFFFFFFFF. You seem to have no upper bound nor any trap on the
alloc overflow. Seems a horrible way to expose the raid commands too ?
qstor_exec_special_cmd doesn't consider datalen overflow. I'm not sure
what stops a lot of parallel callers here but not sure if thats fixed by
the command queueing ?
Correct ioctl return is -ENOTTY for unknown (thats a mistake still in
many existing drivers so no suprise its still being copied)
The ioremap_nocache should just be ioremap I believe
Announces for the driver should be KERN_INFO IMHO
Reset code seems to be spending a lot of time with irq's off ?
qstor_dealloc_device checks dev->id != NULL - kfree(NULL) is an allowed
no-op btw.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/