PROBLEM: Oops in 2.6.3 with lots of SG_IO activity

DISCUSSION Brian King 1 participant 1 message Mar 18, 2004

Subsystems (2)

SCSI SCSI SG driver

lore

Share & tools

lore

Brian King

author

2004-03-18 21:49 UTC #0 lore raw

James,

Attached is a patch to fix an oops in sg_cmd_done. Please apply.

Thanks

-Brian

-------- Original Message --------
Subject: Re: PROBLEM: Oops in 2.6.3 with lots of SG_IO activity
Date: Wed, 10 Mar 2004 10:24:39 -0600
From: Brian King <[email protected]>
To: [email protected]
CC: [email protected], [email protected]
References: <[email protected]> <[email protected]> <[email protected]> <[email protected]>

Quoted 7 lines · Douglas Gilbert

Brian,
Thanks for this test code. I don't follow the "run disk exercisers" bit. BTW iprinit seg faulted when the sg module wasn't loaded
Your patch widens the srp->done window and re-orders kill_fasync() and wake_up_interruptible(). Are they both needed? If not which one is critical?

I need both for my testcase to run clean. sg_cmd_done cannot touch sfp once srp->done is set.

Quoted 3 lines

Anyway I'm happy to go ahead with the patch (posted by you a little while later on the lsml). Having just moved accommodation my equipment still needs more setting up. I have a sym53c8xx HBA.

Thanks. Once again, here is the patch. James, please apply.

Signature · 3 lines

Brian King
eServer Storage I/O
IBM Linux Technology Center

The patch fixes a race condition in sg_cmd_done that results in an oops.

---

drivers/scsi/sg.c renamed from drivers/scsi/sg.c~sg_cmd_done_oops +2 -2

diff -puN drivers/scsi/sg.c~sg_cmd_done_oops drivers/scsi/sg.c
@@ -1256,7 +1256,6 @@ sg_cmd_done(Scsi_Cmnd * SCpnt)
 	SRpnt->sr_request->rq_disk = NULL; /* "sg" _disowns_ request blk */
 
 	srp->my_cmdp = NULL;
-	srp->done = 1;
 
 	SCSI_LOG_TIMEOUT(4, printk("sg_cmd_done: %s, pack_id=%d, res=0x%x\n",
 		sdp->disk->disk_name, srp->header.pack_id, (int) SRpnt->sr_result));

@@ -1312,8 +1311,9 @@ sg_cmd_done(Scsi_Cmnd * SCpnt)
 	}
 	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);
+		srp->done = 1;
+		wake_up_interruptible(&sfp->read_wait);
 	}
 }