* Re: [patch] ips.c spin lock 64 bit issues
@ 2001-08-16 12:23 ServeRAID For Linux
2001-08-16 12:48 ` Richard B. Johnson
0 siblings, 1 reply; 5+ messages in thread
From: ServeRAID For Linux @ 2001-08-16 12:23 UTC (permalink / raw)
To: jes; +Cc: alan, linux-kernel, torvalds
That's only the tip of the iceberg for the "variable casting" and other
changes that are required to make the ips driver completely safe on IA64.
I have made all the changes necessary and we are currently in test mode
with the next release of ips ( Version 4.80.xx ), which will be completely
64-bit safe. I hope to make it available to all sometime in September,
when we have completed our testing with it on both 32 and 64 bit Linux.
jes@trained-mo
nkey.org To: ServeRAID For Linux/Raleigh/IBM@IBMUS
cc: alan@redhat.com, torvalds@transmeta.com,
08/16/2001 linux-kernel@vger.kernel.org
12:11 AM Subject: [patch] ips.c spin lock 64 bit issues
Hi
I noticed the ips.c driver used 32 bit data types for it's cpu_flags
variables passed to spin_lock_irqsave() etc. which isn't safe on 64
systems.
Here is a patch that solves the problem (think I got them all).
Cheers
Jes
--- drivers/scsi/ips.c~ Sat May 19 20:43:06 2001
+++ drivers/scsi/ips.c Thu Aug 16 00:06:57 2001
@@ -1416,7 +1416,7 @@
ips_eh_reset(Scsi_Cmnd *SC) {
int ret;
int i;
- u32 cpu_flags;
+ unsigned long cpu_flags;
ips_ha_t *ha;
ips_scb_t *scb;
ips_copp_wait_item_t *item;
@@ -1607,7 +1607,7 @@
int
ips_queue(Scsi_Cmnd *SC, void (*done) (Scsi_Cmnd *)) {
ips_ha_t *ha;
- u32 cpu_flags;
+ unsigned long cpu_flags;
DECLARE_MUTEX_LOCKED(sem);
METHOD_TRACE("ips_queue", 1);
@@ -1854,7 +1854,7 @@
void
do_ipsintr(int irq, void *dev_id, struct pt_regs *regs) {
ips_ha_t *ha;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("do_ipsintr", 2);
@@ -1909,7 +1909,7 @@
ips_scb_t *scb;
IPS_STATUS cstatus;
int intrstatus;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_intr", 2);
@@ -1981,7 +1981,7 @@
ips_scb_t *scb;
IPS_STATUS cstatus;
int intrstatus;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_intr_morpheus", 2);
@@ -3548,8 +3548,8 @@
ips_copp_wait_item_t *item;
int ret;
int intr_status;
- u32 cpu_flags;
- u32 cpu_flags2;
+ unsigned long cpu_flags;
+ unsigned long cpu_flags2;
METHOD_TRACE("ips_next", 1);
@@ -4403,7 +4403,7 @@
static void
ips_done(ips_ha_t *ha, ips_scb_t *scb) {
int ret;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_done", 1);
@@ -5520,7 +5520,7 @@
static ips_scb_t *
ips_getscb(ips_ha_t *ha) {
ips_scb_t *scb;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_getscb", 1);
@@ -5554,7 +5554,7 @@
/****************************************************************************/
static void
ips_freescb(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_freescb", 1);
@@ -5967,7 +5967,7 @@
static int
ips_reset_copperhead(ips_ha_t *ha) {
int reset_counter;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_copperhead", 1);
@@ -6012,7 +6012,7 @@
static int
ips_reset_copperhead_memio(ips_ha_t *ha) {
int reset_counter;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_copperhead_memio", 1);
@@ -6058,7 +6058,7 @@
ips_reset_morpheus(ips_ha_t *ha) {
int reset_counter;
u8 junk;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_morpheus", 1);
@@ -6237,7 +6237,7 @@
ips_issue_copperhead(ips_ha_t *ha, ips_scb_t *scb) {
u32 TimeOut;
u16 val;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_copperhead", 1);
@@ -6300,7 +6300,7 @@
ips_issue_copperhead_memio(ips_ha_t *ha, ips_scb_t *scb) {
u32 TimeOut;
u32 val;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_copperhead_memio", 1);
@@ -6361,7 +6361,7 @@
/****************************************************************************/
static int
ips_issue_i2o(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_i2o", 1);
@@ -6401,7 +6401,7 @@
/****************************************************************************/
static int
ips_issue_i2o_memio(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_i2o_memio", 1);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] ips.c spin lock 64 bit issues
2001-08-16 12:23 [patch] ips.c spin lock 64 bit issues ServeRAID For Linux
@ 2001-08-16 12:48 ` Richard B. Johnson
2001-08-16 12:55 ` Jes Sorensen
2001-08-16 12:57 ` Alan Cox
0 siblings, 2 replies; 5+ messages in thread
From: Richard B. Johnson @ 2001-08-16 12:48 UTC (permalink / raw)
To: ServeRAID For Linux; +Cc: jes, alan, linux-kernel, torvalds
On Thu, 16 Aug 2001, ServeRAID For Linux wrote:
> That's only the tip of the iceberg for the "variable casting" and other
> changes that are required to make the ips driver completely safe on IA64.
> I have made all the changes necessary and we are currently in test mode
> with the next release of ips ( Version 4.80.xx ), which will be completely
> 64-bit safe. I hope to make it available to all sometime in September,
> when we have completed our testing with it on both 32 and 64 bit Linux.
>
I don't think that "unsigned long" is the correct data type.
Isn't there a data type that means "the largest unsigned integer type
that fits into a register on the target..."? I was told that that's what
"size_t" means. If so, all the flags variables <everywhere> should be
changed to this type. If not, then somebody should define a "flags_t"
type. With the new 64-bit machines, this is going to bite over and
over again until something like this is done.
Cheers,
Dick Johnson
Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).
I was going to compile a list of innovations that could be
attributed to Microsoft. Once I realized that Ctrl-Alt-Del
was handled in the BIOS, I found that there aren't any.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] ips.c spin lock 64 bit issues
2001-08-16 12:48 ` Richard B. Johnson
@ 2001-08-16 12:55 ` Jes Sorensen
2001-08-16 12:57 ` Alan Cox
1 sibling, 0 replies; 5+ messages in thread
From: Jes Sorensen @ 2001-08-16 12:55 UTC (permalink / raw)
To: root; +Cc: ServeRAID For Linux, alan, linux-kernel
>>>>> "Richard" == Richard B Johnson <root@chaos.analogic.com> writes:
Richard> I don't think that "unsigned long" is the correct data type.
Richard> Isn't there a data type that means "the largest unsigned
Richard> integer type that fits into a register on the target..."? I was
Richard> told that that's what "size_t" means. If so, all the flags
Richard> variables <everywhere> should be changed to this type. If not,
Richard> then somebody should define a "flags_t" type. With the new
Richard> 64-bit machines, this is going to bite over and over again
Richard> until something like this is done.
Face it, unsigned long *is* the correct data type. The API has been
specified like this for years, no reason to change it. long us
guaranteed to be able to hold a pointer on Linux, ie. the largest
natural data type.
And no, you are more likely to find a 32 bit size_t on some systems and
we do not need yet another obscure data type flags_t when unsigned long
does the job fine as it is.
Jes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] ips.c spin lock 64 bit issues
2001-08-16 12:48 ` Richard B. Johnson
2001-08-16 12:55 ` Jes Sorensen
@ 2001-08-16 12:57 ` Alan Cox
1 sibling, 0 replies; 5+ messages in thread
From: Alan Cox @ 2001-08-16 12:57 UTC (permalink / raw)
To: root; +Cc: ServeRAID For Linux, jes, alan, linux-kernel, torvalds
> I don't think that "unsigned long" is the correct data type.
Wrong.
> Isn't there a data type that means "the largest unsigned integer type
> that fits into a register on the target..."? I was told that that's what
But we don't care. The flags is defined as unsigned long.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch] ips.c spin lock 64 bit issues
@ 2001-08-16 4:11 jes
0 siblings, 0 replies; 5+ messages in thread
From: jes @ 2001-08-16 4:11 UTC (permalink / raw)
To: ipslinux; +Cc: alan, torvalds, linux-kernel
Hi
I noticed the ips.c driver used 32 bit data types for it's cpu_flags
variables passed to spin_lock_irqsave() etc. which isn't safe on 64
systems.
Here is a patch that solves the problem (think I got them all).
Cheers
Jes
--- drivers/scsi/ips.c~ Sat May 19 20:43:06 2001
+++ drivers/scsi/ips.c Thu Aug 16 00:06:57 2001
@@ -1416,7 +1416,7 @@
ips_eh_reset(Scsi_Cmnd *SC) {
int ret;
int i;
- u32 cpu_flags;
+ unsigned long cpu_flags;
ips_ha_t *ha;
ips_scb_t *scb;
ips_copp_wait_item_t *item;
@@ -1607,7 +1607,7 @@
int
ips_queue(Scsi_Cmnd *SC, void (*done) (Scsi_Cmnd *)) {
ips_ha_t *ha;
- u32 cpu_flags;
+ unsigned long cpu_flags;
DECLARE_MUTEX_LOCKED(sem);
METHOD_TRACE("ips_queue", 1);
@@ -1854,7 +1854,7 @@
void
do_ipsintr(int irq, void *dev_id, struct pt_regs *regs) {
ips_ha_t *ha;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("do_ipsintr", 2);
@@ -1909,7 +1909,7 @@
ips_scb_t *scb;
IPS_STATUS cstatus;
int intrstatus;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_intr", 2);
@@ -1981,7 +1981,7 @@
ips_scb_t *scb;
IPS_STATUS cstatus;
int intrstatus;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_intr_morpheus", 2);
@@ -3548,8 +3548,8 @@
ips_copp_wait_item_t *item;
int ret;
int intr_status;
- u32 cpu_flags;
- u32 cpu_flags2;
+ unsigned long cpu_flags;
+ unsigned long cpu_flags2;
METHOD_TRACE("ips_next", 1);
@@ -4403,7 +4403,7 @@
static void
ips_done(ips_ha_t *ha, ips_scb_t *scb) {
int ret;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_done", 1);
@@ -5520,7 +5520,7 @@
static ips_scb_t *
ips_getscb(ips_ha_t *ha) {
ips_scb_t *scb;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_getscb", 1);
@@ -5554,7 +5554,7 @@
/****************************************************************************/
static void
ips_freescb(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_freescb", 1);
@@ -5967,7 +5967,7 @@
static int
ips_reset_copperhead(ips_ha_t *ha) {
int reset_counter;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_copperhead", 1);
@@ -6012,7 +6012,7 @@
static int
ips_reset_copperhead_memio(ips_ha_t *ha) {
int reset_counter;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_copperhead_memio", 1);
@@ -6058,7 +6058,7 @@
ips_reset_morpheus(ips_ha_t *ha) {
int reset_counter;
u8 junk;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_reset_morpheus", 1);
@@ -6237,7 +6237,7 @@
ips_issue_copperhead(ips_ha_t *ha, ips_scb_t *scb) {
u32 TimeOut;
u16 val;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_copperhead", 1);
@@ -6300,7 +6300,7 @@
ips_issue_copperhead_memio(ips_ha_t *ha, ips_scb_t *scb) {
u32 TimeOut;
u32 val;
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_copperhead_memio", 1);
@@ -6361,7 +6361,7 @@
/****************************************************************************/
static int
ips_issue_i2o(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_i2o", 1);
@@ -6401,7 +6401,7 @@
/****************************************************************************/
static int
ips_issue_i2o_memio(ips_ha_t *ha, ips_scb_t *scb) {
- u32 cpu_flags;
+ unsigned long cpu_flags;
METHOD_TRACE("ips_issue_i2o_memio", 1);
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2001-08-16 12:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-08-16 12:23 [patch] ips.c spin lock 64 bit issues ServeRAID For Linux
2001-08-16 12:48 ` Richard B. Johnson
2001-08-16 12:55 ` Jes Sorensen
2001-08-16 12:57 ` Alan Cox
-- strict thread matches above, loose matches on Subject: below --
2001-08-16 4:11 jes
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox