mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: <jes@trained-monkey.org>
To: linux@3ware.com
Cc: alan@redhat.com, torvalds@transmeta.com,
	linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: [patch] 3Ware 64 bit locking issues
Date: Thu, 16 Aug 2001 00:21:23 -0400	[thread overview]
Message-ID: <200108160421.f7G4LNJ19413@savage.trained-monkey.org> (raw)

Hi

The 3Ware driver was using 32 bit data types for cpu flags arguments
to spin_lock_irqsave which isn't safe. Here is a patch, which also
gets rid of the redundant flag saving in one instance in the interrupt
handler where the flags had just been saved anyway.

I also noticed that the driver does spin_lock() in the error handling
functions, without disabling interrupts on the local cpu. Is this
safe? Ie. are the error handling functions guaranteed to be called
from interrupt context or with interrupts disabled? Note that I didn't
make any changes to this bit.

Jes

--- drivers/scsi/3w-xxxx.c~	Wed Jun 27 20:10:55 2001
+++ drivers/scsi/3w-xxxx.c	Thu Aug 16 00:17:00 2001
@@ -1202,15 +1202,14 @@
 	int do_attention_interrupt=0;
 	int do_host_interrupt=0;
 	int do_command_interrupt=0;
-	int flags = 0;
-	int flags2 = 0;
+	unsigned long flags = 0;
 	TW_Command *command_packet;
 	if (test_and_set_bit(TW_IN_INTR, &tw_dev->flags))
 		return;
 	spin_lock_irqsave(&io_request_lock, flags);
 
 	if (tw_dev->tw_pci_dev->irq == irq) {
-		spin_lock_irqsave(&tw_dev->tw_lock, flags2);
+		spin_lock(&tw_dev->tw_lock);
 		dprintk(KERN_NOTICE "3w-xxxx: tw_interrupt()\n");
 
 		/* Read the registers */
@@ -1349,7 +1348,7 @@
 				}
 			}
 		}
-		spin_unlock_irqrestore(&tw_dev->tw_lock, flags2);
+		spin_unlock(&tw_dev->tw_lock);
 	}
 	spin_unlock_irqrestore(&io_request_lock, flags);
 	clear_bit(TW_IN_INTR, &tw_dev->flags);
@@ -1918,7 +1917,7 @@
 	unsigned char *command = SCpnt->cmnd;
 	int request_id = 0;
 	int error = 0;
-	int flags = 0;
+	unsigned long flags = 0;
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)SCpnt->host->hostdata;
 
 	if (tw_dev == NULL) {

             reply	other threads:[~2001-08-16  4:23 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-08-16  4:21 jes [this message]
     [not found] <53B208BD9A7FD311881A009027B6BBFB9EAE47@siamese>
2001-08-22 19:04 ` Jes Sorensen
2001-08-23  6:59   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200108160421.f7G4LNJ19413@savage.trained-monkey.org \
    --to=jes@trained-monkey.org \
    --cc=alan@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux@3ware.com \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®