2.6.1: media change check fails for busy unplugged device

2.6.1: media change check fails for busy unplugged device

[Andrey Borzenkov]

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

- sdev->online still remains TRUE

so nothing ever tells sd_media_changed that device is gone.

this effectively prevents supermount from noticing that it needs remount 
subfs.

What is the right place to catch those deleted device? For now I add this as 
part of supermount patch, but it looks too ugly (and in the wrong place):

--- linux-2.6.1/drivers/scsi/sd.c       29 Dec 2003 17:20:12 -0000
+++ linux-2.6.1/drivers/scsi/sd.c       18 Jan 2004 18:20:54 -0000
@@ -620,6 +622,16 @@ static int sd_media_changed(struct gendi
                 goto not_present;

        /*
+        * FIXME HACK
+        * busy device that is unplugged is SDEV_DEL but online and ioctl
+        * does not return any error. Oh well, it is likely layering
+        * violation but for now it enables media checks for supermount
+        */
+
+       if (sdp->sdev_state == SDEV_DEL)
+               goto not_present;
+
+       /*
         * For removable scsi disk we have to recognise the presence
         * of a disk in the drive. This is kept in the struct scsi_disk
         * struct and tested at open !  Daniel Roche (dan@lectra.fr)
 
TIA

-andrey

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

[Mike Anderson]

Andrey Borzenkov [arvidjaar@mail.ru] 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.

We need a change in scsi_wait_req to differentiate that we where not woken
up from scsi_wait_done, but from end_that_request_last.

One way would be to check if rq_status == RQ_SCSI_DONE. I did not see
anything on the request to indicate it was BLKPREP_KILL'd.

James has worked more on the scsi_prep_fn so maybe he has another
suggestion.

-andmike
--
Michael Anderson
andmike@us.ibm.com

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

[Andrey Borzenkov]

On Tuesday 20 January 2004 02:36, Mike Anderson wrote:
> Andrey Borzenkov [arvidjaar@mail.ru] 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.

thank you

-andrey

> We need a change in scsi_wait_req to differentiate that we where not woken
> up from scsi_wait_done, but from end_that_request_last.
>
> One way would be to check if rq_status == RQ_SCSI_DONE. I did not see
> anything on the request to indicate it was BLKPREP_KILL'd.
>
> James has worked more on the scsi_prep_fn so maybe he has another
> suggestion.
>
> -andmike
> --
> Michael Anderson
> andmike@us.ibm.com

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

[Mike Anderson]

Andrey Borzenkov [arvidjaar@mail.ru] wrote:
> On Tuesday 20 January 2004 02:36, Mike Anderson wrote:
> > Andrey Borzenkov [arvidjaar@mail.ru] 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@us.ibm.com

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@us.ibm.com>
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);
 }

_

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

[James Bottomley]

On Thu, 2004-01-29 at 04:30, Mike Anderson wrote:
> 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?).

Yes, I've just been extremely reticent about doing it because it
complicates the state model.  However, I'll get off my backside and see
if I can come up with a model update including it.

James