Re: 2.6.1: media change check fails for busy unplugged device

[Mike Anderson]

Andrey Borzenkov [arvidjaar@xxxxxxx] wrote:
> On Tuesday 20 January 2004 02:36, Mike Anderson wrote:
> > Andrey Borzenkov [arvidjaar@xxxxxxx] wrote:
> > > If we unplug busy device (consider mounted USB stick) media change check
> > > always returns true (no media change happened). It happens because
> > >
> > > - device state is set to SDEV_DEL and scsi_prep_fn silently kills any
> > > request including TEST_UNIT_READY sent by sd_media_changed without
> > > propagating any information back to caller
> >
> > The silently kill would appear to be the issue. The addition of any
> > number of additional checks prior to calling scsi_ioctl would not ensure
> > that as soon as the last check is done the device state has not changed.
> >
> 
> But why sdev->online remains set after device has been deleted? It is 
> definitely cannot accept any command after that point?
> 
> Is it possible to clear sdev->online in scsi_remove_device? That would solve 
> the problem.
> 

The sdev->online flag was left as is when the device state model was
update as we where overloading the meaning on this flag. I believe James
last statement on this was that we need to merge this in at some point
(correct James?).

As I previously stated above no flag checking will work in this case
(you can make the race smaller) as it can go SDEV_DEL or offline false
after all kinds of checks, but prior to running the command. The
scsi_wait_req needs to handle the case that it might have been woken up
by someone else beside scsi_wait_done. One way to do this would be the
patch below. I did some testing using a ioctl to the sd device that had
been deleted. The patch changed the result return by the ioctl from
success to failure (It should be noted that there is some odd behavior
in the ioctl_internal_command in that it will not return negative errno
values but instead returns the result of the command which will be
positive under errors). We probably need to come up with a better method
than the test in the patch to determine that a request was killed, but
it should be valid enough to test to see if it solves your issue.

-andmike
--
Michael Anderson
andmike@xxxxxxxxxx

DESC
If a request is sent through scsi_wait_req the function may be woken up
from the completion by a function other than scsi_wait_done. This can
happen as a result of cases that return BLKPREP_KILL in the scsi_prep_fn
function.

author: Mike Anderson <andmike@xxxxxxxxxx>
patch_version: Thu Jan 29 09:03:44 UTC 2004
EDESC


 patched-2.6-andmike/drivers/scsi/scsi_lib.c |    2 ++
 1 files changed, 2 insertions(+)

diff -puN drivers/scsi/scsi_lib.c~sdev_del_scsi_wait_req drivers/scsi/scsi_lib.c
--- patched-2.6/drivers/scsi/scsi_lib.c~sdev_del_scsi_wait_req	Mon Jan 26 18:31:45 2004
+++ patched-2.6-andmike/drivers/scsi/scsi_lib.c	Mon Jan 26 18:50:51 2004
@@ -238,6 +238,8 @@ void scsi_wait_req(struct scsi_request *
 	generic_unplug_device(sreq->sr_device->request_queue);
 	wait_for_completion(&wait);
 	sreq->sr_request->waiting = NULL;
+	if (sreq->sr_request->rq_status != RQ_SCSI_DONE)
+		sreq->sr_result |= (DRIVER_ERROR << 24);
 
 	__scsi_release_request(sreq);
 }

_
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/