* removable device: can't detect disk changes @ 2008-08-12 13:08 Giuliano Pochini 2008-08-18 19:52 ` Giuliano Pochini 0 siblings, 1 reply; 5+ messages in thread From: Giuliano Pochini @ 2008-08-12 13:08 UTC (permalink / raw) To: linux-scsi Fujitsu magneto-optical drive, Adaptec 29160 and Linux Jay 2.6.26 #7 SMP Sun Aug 10 18:34:22 CEST 2008 ppc 7455, altivec supported PowerMac3,6 GNU/Linux When I insert a disk and I mount it, scsi_test_unit_ready() is called and the do-while loop gets sshdr->sense_key == UNIT_ATTENTION in the first cycle and 0 in the second one. So the if below misses the UNIT_ATTENTION and sdev->changed = 1 is not executed. At this point bad things can happen... I'm not sure how to fix this. Any clue ? -- Giuliano. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: removable device: can't detect disk changes 2008-08-12 13:08 removable device: can't detect disk changes Giuliano Pochini @ 2008-08-18 19:52 ` Giuliano Pochini 2008-08-18 20:58 ` James Bottomley 0 siblings, 1 reply; 5+ messages in thread From: Giuliano Pochini @ 2008-08-18 19:52 UTC (permalink / raw) To: linux-scsi On Tue, 12 Aug 2008 15:08:14 +0200 Giuliano Pochini <pochini@shiny.it> wrote: > > Fujitsu magneto-optical drive, Adaptec 29160 and > Linux Jay 2.6.26 #7 SMP Sun Aug 10 18:34:22 CEST 2008 ppc 7455, altivec supported PowerMac3,6 GNU/Linux > > When I insert a disk and I mount it, scsi_test_unit_ready() is called and > the do-while loop gets sshdr->sense_key == UNIT_ATTENTION in the first > cycle and 0 in the second one. So the if below misses the UNIT_ATTENTION > and sdev->changed = 1 is not executed. At this point bad things can > happen... I'm not sure how to fix this. Any clue ? Ok... what about this patch ? The while() condition also checks for NOT_READY. For removable devices sdev->changed is set when not_ready is detected (not only if it occurs in the last cycle). If works fine here, but since I don't know the SCSI subsystem this patch may be wrong. Signed-off-by: Giuliano Pochini <pochini@shiny.it> --- scsi_lib.c__orig 2008-08-18 21:11:45.000000000 +0200 +++ scsi_lib.c 2008-08-18 21:26:56.000000000 +0200 @@ -1961,7 +1961,7 @@ scsi_test_unit_ready(struct scsi_device TEST_UNIT_READY, 0, 0, 0, 0, 0, }; struct scsi_sense_hdr *sshdr; - int result; + int result, not_ready; if (!sshdr_external) sshdr = kzalloc(sizeof(*sshdr), GFP_KERNEL); @@ -1972,23 +1972,19 @@ scsi_test_unit_ready(struct scsi_device do { result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr, timeout, retries); - } while ((driver_byte(result) & DRIVER_SENSE) && - sshdr && sshdr->sense_key == UNIT_ATTENTION && - --retries); + not_ready = (driver_byte(result) & DRIVER_SENSE) && sshdr && + ((sshdr->sense_key == UNIT_ATTENTION) || + (sshdr->sense_key == NOT_READY)); + if (not_ready && scsi_sense_valid(sshdr) && sdev->removable) + sdev->changed = 1; + } while (not_ready && --retries); if (!sshdr) /* could not allocate sense buffer, so can't process it */ return result; - if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) { - - if ((scsi_sense_valid(sshdr)) && - ((sshdr->sense_key == UNIT_ATTENTION) || - (sshdr->sense_key == NOT_READY))) { - sdev->changed = 1; + if (not_ready && scsi_sense_valid(sshdr) && sdev->removable) result = 0; - } - } if (!sshdr_external) kfree(sshdr); return result; -- Giuliano. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: removable device: can't detect disk changes 2008-08-18 19:52 ` Giuliano Pochini @ 2008-08-18 20:58 ` James Bottomley 2008-08-19 22:58 ` Giuliano Pochini 0 siblings, 1 reply; 5+ messages in thread From: James Bottomley @ 2008-08-18 20:58 UTC (permalink / raw) To: Giuliano Pochini; +Cc: linux-scsi On Mon, 2008-08-18 at 21:52 +0200, Giuliano Pochini wrote: > On Tue, 12 Aug 2008 15:08:14 +0200 > Giuliano Pochini <pochini@shiny.it> wrote: > > > > > Fujitsu magneto-optical drive, Adaptec 29160 and > > Linux Jay 2.6.26 #7 SMP Sun Aug 10 18:34:22 CEST 2008 ppc 7455, altivec supported PowerMac3,6 GNU/Linux > > > > When I insert a disk and I mount it, scsi_test_unit_ready() is called and > > the do-while loop gets sshdr->sense_key == UNIT_ATTENTION in the first > > cycle and 0 in the second one. So the if below misses the UNIT_ATTENTION > > and sdev->changed = 1 is not executed. At this point bad things can > > happen... I'm not sure how to fix this. Any clue ? > > Ok... what about this patch ? The while() condition also checks for > NOT_READY. For removable devices sdev->changed is set when not_ready is > detected (not only if it occurs in the last cycle). > If works fine here, but since I don't know the SCSI subsystem this patch > may be wrong. Well done! Almost ... apparently what needs to happen is what we're currently doing in sr_test_unit_ready. There's no need to keep checking for NOT_READY ... that isn't a volatile condition like UNIT ATTENTION and driver_byte(result) & DRIVER_SENSE is equivalent to scsi_sense_valid(sshdr) in most cases. Also, the while loop should exit on a non-UNIT ATTENTION condition (like NOT READY). Does this work? James --- diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index ff5d56b..f2b03b7 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2022,22 +2022,22 @@ scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries, do { result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr, timeout, retries); - } while ((driver_byte(result) & DRIVER_SENSE) && - sshdr && sshdr->sense_key == UNIT_ATTENTION && + if (sdev->removable && sshdr && scsi_sense_valid(sshdr) && + sshdr->sense_key == UNIT_ATTENTION) + sdev->changed = 1; + } while (sshdr && scsi_sense_valid(sshdr) && + sshdr->sense_key == UNIT_ATTENTION && --retries); if (!sshdr) /* could not allocate sense buffer, so can't process it */ return result; - if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) { - - if ((scsi_sense_valid(sshdr)) && - ((sshdr->sense_key == UNIT_ATTENTION) || - (sshdr->sense_key == NOT_READY))) { - sdev->changed = 1; - result = 0; - } + if (sdev->removable && scsi_sense_valid(sshdr) && + (sshdr->sense_key == UNIT_ATTENTION || + sshdr->sense_key == NOT_READY)) { + sdev->changed = 1; + result = 0; } if (!sshdr_external) kfree(sshdr); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: removable device: can't detect disk changes 2008-08-18 20:58 ` James Bottomley @ 2008-08-19 22:58 ` Giuliano Pochini 2008-10-13 20:40 ` Giuliano Pochini 0 siblings, 1 reply; 5+ messages in thread From: Giuliano Pochini @ 2008-08-19 22:58 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi On Mon, 18 Aug 2008 15:58:14 -0500 James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > On Mon, 2008-08-18 at 21:52 +0200, Giuliano Pochini wrote: > > On Tue, 12 Aug 2008 15:08:14 +0200 > > Giuliano Pochini <pochini@shiny.it> wrote: > > > > > > > > Fujitsu magneto-optical drive, Adaptec 29160 and > > > Linux Jay 2.6.26 #7 SMP Sun Aug 10 18:34:22 CEST 2008 ppc 7455, altivec supported PowerMac3,6 GNU/Linux > > > > > > When I insert a disk and I mount it, scsi_test_unit_ready() is called and > > > the do-while loop gets sshdr->sense_key == UNIT_ATTENTION in the first > > > cycle and 0 in the second one. So the if below misses the UNIT_ATTENTION > > > and sdev->changed = 1 is not executed. At this point bad things can > > > happen... I'm not sure how to fix this. Any clue ? > > > > Ok... what about this patch ? The while() condition also checks for > > NOT_READY. For removable devices sdev->changed is set when not_ready is > > detected (not only if it occurs in the last cycle). > > If works fine here, but since I don't know the SCSI subsystem this patch > > may be wrong. > > Well done! Almost ... apparently what needs to happen is what we're > currently doing in sr_test_unit_ready. There's no need to keep checking > for NOT_READY ... that isn't a volatile condition like UNIT ATTENTION > and driver_byte(result) & DRIVER_SENSE is equivalent to > scsi_sense_valid(sshdr) in most cases. Also, the while loop should exit > on a non-UNIT ATTENTION condition (like NOT READY). > > Does this work? > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c [...] Yes, it works fine. This is your patch without the check for sshdr!=NULL because it is already in scsi_sense_valid(). --- drivers/scsi/scsi_lib.c__orig 2008-08-18 21:11:45.000000000 +0200 +++ drivers/scsi/scsi_lib.c 2008-08-19 22:31:41.000000000 +0200 @@ -1972,22 +1972,21 @@ scsi_test_unit_ready(struct scsi_device do { result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr, timeout, retries); - } while ((driver_byte(result) & DRIVER_SENSE) && - sshdr && sshdr->sense_key == UNIT_ATTENTION && - --retries); + if (sdev->removable && scsi_sense_valid(sshdr) && + sshdr->sense_key == UNIT_ATTENTION) + sdev->changed = 1; + } while (scsi_sense_valid(sshdr) && + sshdr->sense_key == UNIT_ATTENTION && --retries); if (!sshdr) /* could not allocate sense buffer, so can't process it */ return result; - if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) { - - if ((scsi_sense_valid(sshdr)) && - ((sshdr->sense_key == UNIT_ATTENTION) || - (sshdr->sense_key == NOT_READY))) { - sdev->changed = 1; - result = 0; - } + if (sdev->removable && scsi_sense_valid(sshdr) && + (sshdr->sense_key == UNIT_ATTENTION || + sshdr->sense_key == NOT_READY)) { + sdev->changed = 1; + result = 0; } if (!sshdr_external) kfree(sshdr); -- Giuliano. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: removable device: can't detect disk changes 2008-08-19 22:58 ` Giuliano Pochini @ 2008-10-13 20:40 ` Giuliano Pochini 0 siblings, 0 replies; 5+ messages in thread From: Giuliano Pochini @ 2008-10-13 20:40 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi On Wed, 20 Aug 2008 00:58:13 +0200 Giuliano Pochini <pochini@shiny.it> wrote: > On Mon, 18 Aug 2008 15:58:14 -0500 > James Bottomley <James.Bottomley@HansenPartnership.com> wrote: > > > On Mon, 2008-08-18 at 21:52 +0200, Giuliano Pochini wrote: > > > On Tue, 12 Aug 2008 15:08:14 +0200 > > > Giuliano Pochini <pochini@shiny.it> wrote: > > > > > > > > > > > Fujitsu magneto-optical drive, Adaptec 29160 and > > > > Linux Jay 2.6.26 #7 SMP Sun Aug 10 18:34:22 CEST 2008 ppc 7455, altivec supported PowerMac3,6 GNU/Linux > > > > > > > > When I insert a disk and I mount it, scsi_test_unit_ready() is called and > > > > the do-while loop gets sshdr->sense_key == UNIT_ATTENTION in the first > > > > cycle and 0 in the second one. So the if below misses the UNIT_ATTENTION > > > > and sdev->changed = 1 is not executed. At this point bad things can > > > > happen... I'm not sure how to fix this. Any clue ? > > > > > > Ok... what about this patch ? The while() condition also checks for > > > NOT_READY. For removable devices sdev->changed is set when not_ready is > > > detected (not only if it occurs in the last cycle). > > > If works fine here, but since I don't know the SCSI subsystem this patch > > > may be wrong. > > > > Well done! Almost ... apparently what needs to happen is what we're > > currently doing in sr_test_unit_ready. There's no need to keep checking > > for NOT_READY ... that isn't a volatile condition like UNIT ATTENTION > > and driver_byte(result) & DRIVER_SENSE is equivalent to > > scsi_sense_valid(sshdr) in most cases. Also, the while loop should exit > > on a non-UNIT ATTENTION condition (like NOT READY). > > > > Does this work? > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > [...] > > Yes, it works fine. This is your patch without the check for sshdr!=NULL > because it is already in scsi_sense_valid(). > > > --- drivers/scsi/scsi_lib.c__orig 2008-08-18 21:11:45.000000000 +0200 > +++ drivers/scsi/scsi_lib.c 2008-08-19 22:31:41.000000000 +0200 > @@ -1972,22 +1972,21 @@ scsi_test_unit_ready(struct scsi_device > do { > result = scsi_execute_req(sdev, cmd, DMA_NONE, NULL, 0, sshdr, > timeout, retries); > - } while ((driver_byte(result) & DRIVER_SENSE) && > - sshdr && sshdr->sense_key == UNIT_ATTENTION && > - --retries); > + if (sdev->removable && scsi_sense_valid(sshdr) && > + sshdr->sense_key == UNIT_ATTENTION) > + sdev->changed = 1; > + } while (scsi_sense_valid(sshdr) && > + sshdr->sense_key == UNIT_ATTENTION && --retries); > > if (!sshdr) > /* could not allocate sense buffer, so can't process it */ > return result; > > - if ((driver_byte(result) & DRIVER_SENSE) && sdev->removable) { > - > - if ((scsi_sense_valid(sshdr)) && > - ((sshdr->sense_key == UNIT_ATTENTION) || > - (sshdr->sense_key == NOT_READY))) { > - sdev->changed = 1; > - result = 0; > - } > + if (sdev->removable && scsi_sense_valid(sshdr) && > + (sshdr->sense_key == UNIT_ATTENTION || > + sshdr->sense_key == NOT_READY)) { > + sdev->changed = 1; > + result = 0; > } > if (!sshdr_external) > kfree(sshdr); Will you merge this patch ? If you think there is something wrong, please send me a new patch ASAP. I'm going to sell my removable drive, thus I'll not be able to test it anymore. -- Giuliano. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-10-13 20:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-08-12 13:08 removable device: can't detect disk changes Giuliano Pochini 2008-08-18 19:52 ` Giuliano Pochini 2008-08-18 20:58 ` James Bottomley 2008-08-19 22:58 ` Giuliano Pochini 2008-10-13 20:40 ` Giuliano Pochini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox