Skip to contentSkip to conversation

dont decrement counters from scsi_kill_request

PATCH In review Mike Christie 5 participants 5 messages Jan 4, 2006 SCSI

lore
Share & tools
lore

Mike Christie

author
#0 lore raw

PATCH 1/1 dont decrement counters from scsi_kill_request

This was debugged by [email protected] on the open-iscsi lists.

When we call scsi_kill_request() we have not yet incremented the device and host busy counters so calling __scsi_done from scsi_kill_request ends up doing extra decrements.

This patch has only been compile tested against scsi-misc. I am not sure what the correct blk_complete_barrier_rq values should be. For failures are we supposed to return zero or what is left in the command and what how are we supposed set to indicate a error?

Signed-off-by Mike Christie <[email protected]>
drivers/scsi/scsi_lib.c +12 -4
index a7f3f0c..4b6a9fc 100644
@@ -1514,8 +1514,6 @@ static void scsi_kill_request(struct req
 {
 	struct scsi_cmnd *cmd = req->special;
 
-	blkdev_dequeue_request(req);
-
 	if (unlikely(cmd == NULL)) {
 		printk(KERN_CRIT "impossible request in %s.\n",
 				 __FUNCTION__);

@@ -1523,9 +1521,19 @@ static void scsi_kill_request(struct req
 	}
 
 	scsi_init_cmd_errh(cmd);
-	cmd->result = DID_NO_CONNECT << 16;
 	atomic_inc(&cmd->device->iorequest_cnt);
-	__scsi_done(cmd);
+
+	blkdev_dequeue_request(req);
+	req->errors = DID_NO_CONNECT << 16;
+
+	if (blk_complete_barrier_rq(q, req, cmd->bufflen  >> 9)); 
+		return;
+
+	scsi_release_buffers(cmd);
+	while (end_that_request_first(req, 0, req->nr_sectors))
+		;
+	end_that_request_last(req);
+	scsi_put_command(cmd);
 }
 
 /*
#1 lore raw

drivers/scsi/scsi_lib.c static void scsi_kill_request(struct req · line 1534

Quoted 2 lines · Mike Christie
drivers/scsi/scsi_lib.c +2 -0
+	while (end_that_request_first(req, 0, req->nr_sectors))
+		;

This should be something like

while (...)

	{}

so it is absolutely clear that the empty body is intentional.

Eike

#2 lore raw
Quoted 11 lines · Rolf Eike Beer

drivers/scsi/scsi_lib.c static void scsi_kill_request(struct req · line 1534

Quoted 2 lines · Mike Christie
drivers/scsi/scsi_lib.c +2 -0
+	while (end_that_request_first(req, 0, req->nr_sectors))
+		;

This should be something like

while (...)

	{}

so it is absolutely clear that the empty body is intentional.

Please, do not inflict that ugliness on everyone. FWIW, it's less idiomatic than normal use of ; and personally I'd parse it as "somebody had fscked up using editor", not as "wow, they _really_ meant it to be an empty body".

#3 lore raw
Quoted 10 lines · Al Viro
Quoted 9 lines · Rolf Eike Beer

drivers/scsi/scsi_lib.c static void scsi_kill_request(struct req · line 1534

Quoted 2 lines · Mike Christie
drivers/scsi/scsi_lib.c +2 -0
+	while (end_that_request_first(req, 0, req->nr_sectors))
+		;

This should be something like

while (...)

	{}
Quoted 6 lines
Quoted 1 line

so it is absolutely clear that the empty body is intentional.

Please, do not inflict that ugliness on everyone. FWIW, it's less idiomatic than normal use of ; and personally I'd parse it as "somebody had fscked up using editor", not as "wow, they _really_ meant it to be an empty body".

Using only ; on a single line looks like some code left over from removing code... Ok, so let's agree to disagree. Since coding style says the opening brace should be on the same line as the while, would it be better to put everthing in one line to make clear what's going on?

Eike

#4 lore raw
Quoted 13 lines · Rolf Eike Beer
Quoted 12 lines · Al Viro
Quoted 10 lines · Rolf Eike Beer

drivers/scsi/scsi_lib.c static void scsi_kill_request(struct req · line 1534

Quoted 2 lines · Mike Christie
drivers/scsi/scsi_lib.c +2 -0
+	while (end_that_request_first(req, 0, req->nr_sectors))
+		;

This should be something like

while (...)

	{}

so it is absolutely clear that the empty body is intentional.

Quoted 7 lines
Quoted 4 lines

Please, do not inflict that ugliness on everyone. FWIW, it's less idiomatic than normal use of ; and personally I'd parse it as "somebody had fscked up using editor", not as "wow, they _really_ meant it to be an empty body".

Using only ; on a single line looks like some code left over from removing code...

It's a common idiom and above code is copy and pasted from a perfectly fine indented function. So could you please stop this silly discussion now?