scsi_error: fix scsi_eh_lock_door() documentation
PATCH 1/1 scsi_error: fix scsi_eh_lock_door() documentation
Nowadays eh_lock_door_done() uses blk_get_request() instead of scsi_allocate_request().
---
on top of linux-next
drivers/scsi/scsi_error.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/scsi/scsi_error.c +3 -3
@@ -1451,7 +1451,7 @@ static void eh_lock_door_done(struct req* @sdev: SCSI device to prevent medium removal * * Locking: -* We must be called from process context; scsi_allocate_request() +* We must be called from process context; blk_get_request() * may sleep. * * Notes:
@@ -1459,11 +1459,11 @@ static void eh_lock_door_done(struct req* head of the devices request queue, and continue. * * Bugs: -* scsi_allocate_request() may sleep waiting for existing requests to +* blk_get_request() may sleep waiting for existing requests to * be processed. However, since we haven't kicked off any request * processing for this host, this may deadlock. * -* If scsi_allocate_request() fails for what ever reason, we +* If blk_get_request() fails for what ever reason, we * completely forget to lock the door. */ static void scsi_eh_lock_door (struct scsi_device *sdev)
James Bottomley
maintainerQuoted 2 lines · Bartlomiej Zolnierkiewicz
Nowadays
eh_lock_door_done()usesblk_get_request()instead ofscsi_allocate_request().
I appreciate any attempt to clean up the rather parlous state of our documentation. However, unfortunately, this isn't a simple search and replace job ... what you've done here is transform an obviously incorrect set of statements into a more plausible but still incorrect set of statements.
Quoted 12 lines
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>---
on top of linux-nextdrivers/scsi/scsi_error.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)Index:
b/drivers/scsi/scsi_error.c
===================================================================
drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1455
Quoted 6 lines
drivers/scsi/scsi_error.c +1
-1@@ -1451,7 +1451,7 @@ static void eh_lock_door_done(struct req* @sdev: SCSI device to prevent medium removal * * Locking: -* We must be called from process context; scsi_allocate_request() +* We must be called from process context; blk_get_request() * may sleep.
Actually, it may or may not depending upon what the user tells it in the gfp argument, so the best course here is to kill everything after the semicolon.
drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1464
Quoted 9 lines
drivers/scsi/scsi_error.c +1
-1* Notes: @@ -1459,11 +1459,11 @@ static void eh_lock_door_done(struct req* head of the devices request queue, and continue. * * Bugs: -* scsi_allocate_request() may sleep waiting for existing requests to +* blk_get_request() may sleep waiting for existing requests to * be processed. However, since we haven't kicked off any request * processing for this host, this may deadlock.
May have been true for scsi_allocate_request, certainly untrue for blk_get_request, since we've gone to great pains to eliminate these problems.
drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1466
Quoted 3 lines
drivers/scsi/scsi_error.c +1
-1-* If scsi_allocate_request() fails for what ever reason, we +* If blk_get_request() fails for what ever reason, we * completely forget to lock the door.
Misleading, since blk_get_request() cannot fail given the gfp flags we're using.
So the correct way to fix all of this (including a code change to prevent the spurious null check on blk_get_request() return) is below.
James
---
drivers/scsi/scsi_error.c +5 -12
@@ -1451,28 +1451,21 @@ static void eh_lock_door_done(struct request *req, int uptodate)* @sdev: SCSI device to prevent medium removal * * Locking: -* We must be called from process context; scsi_allocate_request() -* may sleep. +* We must be called from process context. * * Notes: * We queue up an asynchronous "ALLOW MEDIUM REMOVAL" request on the * head of the devices request queue, and continue. -* -* Bugs: -* scsi_allocate_request() may sleep waiting for existing requests to -* be processed. However, since we haven't kicked off any request -* processing for this host, this may deadlock. -* -* If scsi_allocate_request() fails for what ever reason, we -* completely forget to lock the door. */ static void scsi_eh_lock_door (struct scsi_device *sdev) {struct request *req; +/* +* blk_get_request with GFP_KERNEL (__GFP_WAIT) sleeps until a +* request becomes available +*/ req =blk_get_request (sdev->request_queue,READ ,GFP_KERNEL ); -if (!req) -return ; req->cmd[0 ] =ALLOW_MEDIUM_REMOVAL ; req->cmd[1 ] =0 ;
Quoted 6 lines · On Sunday 17 May 2009 16:30:48 James Bottomley wrote:
Quoted 2 lines · Bartlomiej Zolnierkiewicz
Nowadays
eh_lock_door_done()usesblk_get_request()instead ofscsi_allocate_request().I appreciate any attempt to clean up the rather parlous state of our documentation. However, unfortunately, this isn't a simple search and
To be honest, I get a very different feeling here...
My two previous obviously correct and trivial patches fixing documentation for scsi_device_lookup_by_target() and scsi_eh_restore_cmnd() seem to be just lost (they were posted on 27th April)...
Quoted 27 lines
replace job ... what you've done here is transform an obviously incorrect set of statements into a more plausible but still incorrect set of statements.
Quoted 12 lines
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>---
on top of linux-nextdrivers/scsi/scsi_error.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)Index:
b/drivers/scsi/scsi_error.c
===================================================================drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1455
Quoted 6 lines
drivers/scsi/scsi_error.c +1
-1@@ -1451,7 +1451,7 @@ static void eh_lock_door_done(struct req* @sdev: SCSI device to prevent medium removal * * Locking: -* We must be called from process context; scsi_allocate_request() +* We must be called from process context; blk_get_request() * may sleep. Actually, it may or may not depending upon what the user tells it in the gfp argument, so the best course here is to kill everything after the semicolon.
Quoted 13 lines
drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1464
Quoted 9 lines
drivers/scsi/scsi_error.c +1
-1* Notes: @@ -1459,11 +1459,11 @@ static void eh_lock_door_done(struct req* head of the devices request queue, and continue. * * Bugs: -* scsi_allocate_request() may sleep waiting for existing requests to +* blk_get_request() may sleep waiting for existing requests to * be processed. However, since we haven't kicked off any request * processing for this host, this may deadlock. May have been true for
scsi_allocate_request, certainly untrue forblk_get_request, since we've gone to great pains to eliminate these problems.
Okay, I recall the trick used there now (AKA "batching" processes)...
Quoted 9 lines
drivers/scsi/scsi_error.c static void eh_lock_door_done(struct req · line 1466
Quoted 3 lines
drivers/scsi/scsi_error.c +1
-1-* If scsi_allocate_request() fails for what ever reason, we +* If blk_get_request() fails for what ever reason, we * completely forget to lock the door. Misleading, since
blk_get_request()cannot fail given the gfp flags we're using.So the correct way to fix all of this (including a code change to prevent the spurious null check on
blk_get_request()return) is below.
Agreed, this version is much better.
Quoted 42 lines
James
---
drivers/scsi/scsi_error.c +5
-12@@ -1451,28 +1451,21 @@ static void eh_lock_door_done(struct request *req, int uptodate)* @sdev: SCSI device to prevent medium removal * * Locking: -* We must be called from process context; scsi_allocate_request() -* may sleep. +* We must be called from process context. * * Notes: * We queue up an asynchronous "ALLOW MEDIUM REMOVAL" request on the * head of the devices request queue, and continue. -* -* Bugs: -* scsi_allocate_request() may sleep waiting for existing requests to -* be processed. However, since we haven't kicked off any request -* processing for this host, this may deadlock. -* -* If scsi_allocate_request() fails for what ever reason, we -* completely forget to lock the door. */ static void scsi_eh_lock_door (struct scsi_device *sdev) {struct request *req; +/* +* blk_get_request with GFP_KERNEL (__GFP_WAIT) sleeps until a +* request becomes available +*/ req =blk_get_request (sdev->request_queue,READ ,GFP_KERNEL ); -if (!req) -return ; req->cmd[0 ] =ALLOW_MEDIUM_REMOVAL ; req->cmd[1 ] =0 ;