* [patch] 2.4 fix for sg driver cmd_done oops
@ 2004-09-30 19:40 Tom Coughlan
2004-09-30 21:53 ` Tony Battersby
0 siblings, 1 reply; 3+ messages in thread
From: Tom Coughlan @ 2004-09-30 19:40 UTC (permalink / raw)
To: SCSI Mailing List; +Cc: petrides, dougg
[-- Attachment #1: Type: text/plain, Size: 331 bytes --]
Brian King posted a patch for 2.6 that fixes a race in sg cmd_done:
http://marc.theaimsgroup.com/?l=linux-scsi&m=108636386217512&w=2
This problem has also been seen on 2.4. The attached patch is a
straight-forward port of Brian's patch to 2.4.28-pre3. Are there any
2.4-specific issues that this does not address?
Thanks,
Tom
[-- Attachment #2: sg_cmd_done_race_2.4.28-pre3.patch --]
[-- Type: text/plain, Size: 2539 bytes --]
--- linux-2.4.28-pre3/drivers/scsi/sg.c.orig
+++ linux-2.4.28-pre3/drivers/scsi/sg.c
@@ -752,6 +752,18 @@ static inline unsigned sg_jif_to_ms(int
}
}
+static int
+sg_srp_done(Sg_request *srp, Sg_fd *sfp)
+{
+ unsigned long iflags;
+ int done;
+
+ read_lock_irqsave(&sfp->rq_list_lock, iflags);
+ done = srp->done;
+ read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
+ return done;
+}
+
static int sg_ioctl(struct inode * inode, struct file * filp,
unsigned int cmd_in, unsigned long arg)
{
@@ -786,7 +798,8 @@ static int sg_ioctl(struct inode * inode
while (1) {
result = 0; /* following macro to beat race condition */
__wait_event_interruptible(sfp->read_wait,
- (sdp->detached || sfp->closed || srp->done), result);
+ (sdp->detached || sfp->closed || sg_srp_done(srp, sfp)),
+ result);
if (sdp->detached)
return -ENODEV;
if (sfp->closed)
@@ -796,7 +809,9 @@ static int sg_ioctl(struct inode * inode
srp->orphan = 1;
return result; /* -ERESTARTSYS because signal hit process */
}
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
srp->done = 2;
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
result = sg_new_read(sfp, (char *)arg, SZ_SG_IO_HDR, srp);
return (result < 0) ? result : 0;
}
@@ -1207,6 +1222,7 @@ static void sg_cmd_done_bh(Scsi_Cmnd * S
Sg_device * sdp = NULL;
Sg_fd * sfp;
Sg_request * srp = NULL;
+ unsigned long iflags;
read_lock(&sg_dev_arr_lock);
if (sg_dev_arr && (dev >= 0)) {
@@ -1316,8 +1332,11 @@ static void sg_cmd_done_bh(Scsi_Cmnd * S
}
if (sfp && srp) {
/* Now wake up any sg_read() that is waiting for this packet. */
- wake_up_interruptible(&sfp->read_wait);
kill_fasync(&sfp->async_qp, SIGPOLL, POLL_IN);
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
+ srp->done = 1;
+ wake_up_interruptible(&sfp->read_wait);
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
}
}
@@ -1507,7 +1526,7 @@ static void sg_detach(Scsi_Device * scsi
tsfp = sfp->nextfp;
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (sfp->closed || (0 == srp->done))
+ if (sfp->closed || (0 == sg_srp_done(srp, sfp)))
sg_finish_rem_req(srp);
}
if (sfp->closed) {
@@ -2425,7 +2444,7 @@ static int sg_remove_sfp(Sg_device * sdp
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (srp->done)
+ if (sg_srp_done(srp, sfp))
sg_finish_rem_req(srp);
else
++dirty;
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [patch] 2.4 fix for sg driver cmd_done oops
2004-09-30 19:40 [patch] 2.4 fix for sg driver cmd_done oops Tom Coughlan
@ 2004-09-30 21:53 ` Tony Battersby
2004-10-04 5:47 ` Douglas Gilbert
0 siblings, 1 reply; 3+ messages in thread
From: Tony Battersby @ 2004-09-30 21:53 UTC (permalink / raw)
To: 'Tom Coughlan', 'SCSI Mailing List'; +Cc: petrides, dougg
[-- Attachment #1: Type: text/plain, Size: 505 bytes --]
> This problem has also been seen on 2.4. The attached patch is a
> straight-forward port of Brian's patch to 2.4.28-pre3. Are there any
> 2.4-specific issues that this does not address?
Try this patch instead, which deletes one line containing "srp->done =
1;" compared to your patch. The 2.6 sg.c moved this line down to fix a
race condition and then later added the locks around it; your patch adds
the line after it was moved but doesn't delete the original line.
Anthony J. Battersby
Cybernetics
[-- Attachment #2: sg_cmd_done_race_2.4.28-pre3.patch --]
[-- Type: application/octet-stream, Size: 2652 bytes --]
--- linux-2.4.28-pre3/drivers/scsi/sg.c.orig Thu Sep 30 17:39:10 2004
+++ linux-2.4.28-pre3/drivers/scsi/sg.c Thu Sep 30 17:39:37 2004
@@ -752,6 +752,18 @@
}
}
+static int
+sg_srp_done(Sg_request *srp, Sg_fd *sfp)
+{
+ unsigned long iflags;
+ int done;
+
+ read_lock_irqsave(&sfp->rq_list_lock, iflags);
+ done = srp->done;
+ read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
+ return done;
+}
+
static int sg_ioctl(struct inode * inode, struct file * filp,
unsigned int cmd_in, unsigned long arg)
{
@@ -786,7 +798,8 @@
while (1) {
result = 0; /* following macro to beat race condition */
__wait_event_interruptible(sfp->read_wait,
- (sdp->detached || sfp->closed || srp->done), result);
+ (sdp->detached || sfp->closed || sg_srp_done(srp, sfp)),
+ result);
if (sdp->detached)
return -ENODEV;
if (sfp->closed)
@@ -796,7 +809,9 @@
srp->orphan = 1;
return result; /* -ERESTARTSYS because signal hit process */
}
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
srp->done = 2;
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
result = sg_new_read(sfp, (char *)arg, SZ_SG_IO_HDR, srp);
return (result < 0) ? result : 0;
}
@@ -1207,6 +1222,7 @@
Sg_device * sdp = NULL;
Sg_fd * sfp;
Sg_request * srp = NULL;
+ unsigned long iflags;
read_lock(&sg_dev_arr_lock);
if (sg_dev_arr && (dev >= 0)) {
@@ -1253,7 +1269,6 @@
SRpnt->sr_request.rq_dev = MKDEV(0, 0); /* "sg" _disowns_ request blk */
srp->my_cmdp = NULL;
- srp->done = 1;
read_unlock(&sg_dev_arr_lock);
SCSI_LOG_TIMEOUT(4, printk("sg...bh: dev=%d, pack_id=%d, res=0x%x\n",
@@ -1316,8 +1331,11 @@
}
if (sfp && srp) {
/* Now wake up any sg_read() that is waiting for this packet. */
- wake_up_interruptible(&sfp->read_wait);
kill_fasync(&sfp->async_qp, SIGPOLL, POLL_IN);
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
+ srp->done = 1;
+ wake_up_interruptible(&sfp->read_wait);
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
}
}
@@ -1507,7 +1525,7 @@
tsfp = sfp->nextfp;
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (sfp->closed || (0 == srp->done))
+ if (sfp->closed || (0 == sg_srp_done(srp, sfp)))
sg_finish_rem_req(srp);
}
if (sfp->closed) {
@@ -2425,7 +2443,7 @@
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (srp->done)
+ if (sg_srp_done(srp, sfp))
sg_finish_rem_req(srp);
else
++dirty;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch] 2.4 fix for sg driver cmd_done oops
2004-09-30 21:53 ` Tony Battersby
@ 2004-10-04 5:47 ` Douglas Gilbert
0 siblings, 0 replies; 3+ messages in thread
From: Douglas Gilbert @ 2004-10-04 5:47 UTC (permalink / raw)
To: tonyb
Cc: 'Tom Coughlan', 'SCSI Mailing List', petrides,
marcelo.tosatti
[-- Attachment #1: Type: text/plain, Size: 651 bytes --]
Tony Battersby wrote:
>>This problem has also been seen on 2.4. The attached patch is a
>>straight-forward port of Brian's patch to 2.4.28-pre3. Are there any
>>2.4-specific issues that this does not address?
>
>
> Try this patch instead, which deletes one line containing "srp->done =
> 1;" compared to your patch. The 2.6 sg.c moved this line down to fix a
> race condition and then later added the locks around it; your patch adds
> the line after it was moved but doesn't delete the original line.
Tom,
Could you confirm whether this patch from Tony fixes the
problem that you reported and if so we can ask Marcelo
to apply it.
Doug Gilbert
[-- Attachment #2: sg_cmd_done_race_2.4.28-pre3.patch --]
[-- Type: text/x-patch, Size: 2564 bytes --]
--- linux-2.4.28-pre3/drivers/scsi/sg.c.orig Thu Sep 30 17:39:10 2004
+++ linux-2.4.28-pre3/drivers/scsi/sg.c Thu Sep 30 17:39:37 2004
@@ -752,6 +752,18 @@
}
}
+static int
+sg_srp_done(Sg_request *srp, Sg_fd *sfp)
+{
+ unsigned long iflags;
+ int done;
+
+ read_lock_irqsave(&sfp->rq_list_lock, iflags);
+ done = srp->done;
+ read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
+ return done;
+}
+
static int sg_ioctl(struct inode * inode, struct file * filp,
unsigned int cmd_in, unsigned long arg)
{
@@ -786,7 +798,8 @@
while (1) {
result = 0; /* following macro to beat race condition */
__wait_event_interruptible(sfp->read_wait,
- (sdp->detached || sfp->closed || srp->done), result);
+ (sdp->detached || sfp->closed || sg_srp_done(srp, sfp)),
+ result);
if (sdp->detached)
return -ENODEV;
if (sfp->closed)
@@ -796,7 +809,9 @@
srp->orphan = 1;
return result; /* -ERESTARTSYS because signal hit process */
}
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
srp->done = 2;
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
result = sg_new_read(sfp, (char *)arg, SZ_SG_IO_HDR, srp);
return (result < 0) ? result : 0;
}
@@ -1207,6 +1222,7 @@
Sg_device * sdp = NULL;
Sg_fd * sfp;
Sg_request * srp = NULL;
+ unsigned long iflags;
read_lock(&sg_dev_arr_lock);
if (sg_dev_arr && (dev >= 0)) {
@@ -1253,7 +1269,6 @@
SRpnt->sr_request.rq_dev = MKDEV(0, 0); /* "sg" _disowns_ request blk */
srp->my_cmdp = NULL;
- srp->done = 1;
read_unlock(&sg_dev_arr_lock);
SCSI_LOG_TIMEOUT(4, printk("sg...bh: dev=%d, pack_id=%d, res=0x%x\n",
@@ -1316,8 +1331,11 @@
}
if (sfp && srp) {
/* Now wake up any sg_read() that is waiting for this packet. */
- wake_up_interruptible(&sfp->read_wait);
kill_fasync(&sfp->async_qp, SIGPOLL, POLL_IN);
+ write_lock_irqsave(&sfp->rq_list_lock, iflags);
+ srp->done = 1;
+ wake_up_interruptible(&sfp->read_wait);
+ write_unlock_irqrestore(&sfp->rq_list_lock, iflags);
}
}
@@ -1507,7 +1525,7 @@
tsfp = sfp->nextfp;
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (sfp->closed || (0 == srp->done))
+ if (sfp->closed || (0 == sg_srp_done(srp, sfp)))
sg_finish_rem_req(srp);
}
if (sfp->closed) {
@@ -2425,7 +2443,7 @@
for (srp = sfp->headrp; srp; srp = tsrp) {
tsrp = srp->nextrp;
- if (srp->done)
+ if (sg_srp_done(srp, sfp))
sg_finish_rem_req(srp);
else
++dirty;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2004-10-04 5:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-30 19:40 [patch] 2.4 fix for sg driver cmd_done oops Tom Coughlan
2004-09-30 21:53 ` Tony Battersby
2004-10-04 5:47 ` Douglas Gilbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).