[PATCH 0/3] scsi: usb-storage: hide errors for five devices

[PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Paul Bolle]

0) Five USB memory sticks I happen to have will generate sets of two
errors when inserted. These sets are either:
    [...] Asking for cache data failed
    [...] Assuming drive cache: write through

or:
    [...] No Caching mode page present
    [...] Assuming drive cache: write through

These errors are generated by the SCSI stack (and printed thrice).

1) These patches try to hide those errors by:
- downgrading one error to a notice; and
- setting the NO_WP_DETECT quirk for these five devices.

2) Review of these patches is appreciated. Please note that the first
patch downgrades the error also for code paths that I apparently never
trigger, so that one especially needs additional thought, testing, etc.
 
3) It's not entirely clear which names one should use in the
UNUSUAL_DEV() macro. I just picked the most neutral names that showed up
for the devices in the logs.

4) Tested on v3.3.4, but applies cleanly to v3.4-rc6.

Paul Bolle (3):
  scsi: downgrade "write through" error to notice
  usb-storage: use NO_WP_DETECT for two devices
  usb-storage: use NO_WP_DETECT for three devices

 drivers/scsi/sd.c                  |    2 +-
 drivers/usb/storage/unusual_devs.h |   22 ++++++++++++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

-- 
1.7.7.6

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Alan Stern]

On Tue, 8 May 2012, Paul Bolle wrote:

> 0) Five USB memory sticks I happen to have will generate sets of two
> errors when inserted. These sets are either:
>     [...] Asking for cache data failed
>     [...] Assuming drive cache: write through
> 
> or:
>     [...] No Caching mode page present
>     [...] Assuming drive cache: write through
> 
> These errors are generated by the SCSI stack (and printed thrice).
> 
> 1) These patches try to hide those errors by:
> - downgrading one error to a notice; and

That's a reasonable thing to do, IMO.

> - setting the NO_WP_DETECT quirk for these five devices.

But that isn't.  These quirks are intended for devices that crash when
they receive the command in question.  They aren't meant to suppress
sending commands to devices that can properly reject them.

Alan Stern

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Paul Bolle]

On Tue, 2012-05-08 at 10:02 -0400, Alan Stern wrote:
> On Tue, 8 May 2012, Paul Bolle wrote: 
> > 1) These patches try to hide those errors by:
> > - downgrading one error to a notice; and
> 
> That's a reasonable thing to do, IMO.
> 
> > - setting the NO_WP_DETECT quirk for these five devices.
> 
> But that isn't.  These quirks are intended for devices that crash when
> they receive the command in question.

Yes, these USB memory sticks don't crash. (They actually seem to work
just fine, something that I perhaps should have emphasized in the commit
descriptions.)

> They aren't meant to suppress sending commands to devices that can
> properly reject them.

Even the sticks that hit "bad_sense" (in sd_read_cache_type(), which I
forgot to mention in the comment descriptions)? Is that not as severe as
it suggests?

Of course, an easy way out would be to downgrade both the "Asking for
cache data failed" and the "No Caching mode page present" errors to
notices. But the SCSI people might disagree with that approach.


Paul Bolle

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Alan Stern]

On Tue, 8 May 2012, Paul Bolle wrote:

> On Tue, 2012-05-08 at 10:02 -0400, Alan Stern wrote:
> > On Tue, 8 May 2012, Paul Bolle wrote: 
> > > 1) These patches try to hide those errors by:
> > > - downgrading one error to a notice; and
> > 
> > That's a reasonable thing to do, IMO.
> > 
> > > - setting the NO_WP_DETECT quirk for these five devices.
> > 
> > But that isn't.  These quirks are intended for devices that crash when
> > they receive the command in question.
> 
> Yes, these USB memory sticks don't crash. (They actually seem to work
> just fine, something that I perhaps should have emphasized in the commit
> descriptions.)
> 
> > They aren't meant to suppress sending commands to devices that can
> > properly reject them.
> 
> Even the sticks that hit "bad_sense" (in sd_read_cache_type(), which I
> forgot to mention in the comment descriptions)? Is that not as severe as
> it suggests?

It means that the device either doesn't support the MODE SENSE command
or it returned useless data.  As a result, we will assume it has a
write-through cache when it might not.

For memory sticks this doesn't matter.  For other devices it might be 
more important (although anything with a working cache should not hit 
this error case).

> Of course, an easy way out would be to downgrade both the "Asking for
> cache data failed" and the "No Caching mode page present" errors to
> notices. But the SCSI people might disagree with that approach.

Well, let's see what they say.

Alan Stern

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Greg Kroah-Hartman]

On Tue, May 08, 2012 at 01:03:17PM -0400, Alan Stern wrote:
> On Tue, 8 May 2012, Paul Bolle wrote:
> 
> > On Tue, 2012-05-08 at 10:02 -0400, Alan Stern wrote:
> > > On Tue, 8 May 2012, Paul Bolle wrote: 
> > > > 1) These patches try to hide those errors by:
> > > > - downgrading one error to a notice; and
> > > 
> > > That's a reasonable thing to do, IMO.
> > > 
> > > > - setting the NO_WP_DETECT quirk for these five devices.
> > > 
> > > But that isn't.  These quirks are intended for devices that crash when
> > > they receive the command in question.
> > 
> > Yes, these USB memory sticks don't crash. (They actually seem to work
> > just fine, something that I perhaps should have emphasized in the commit
> > descriptions.)
> > 
> > > They aren't meant to suppress sending commands to devices that can
> > > properly reject them.
> > 
> > Even the sticks that hit "bad_sense" (in sd_read_cache_type(), which I
> > forgot to mention in the comment descriptions)? Is that not as severe as
> > it suggests?
> 
> It means that the device either doesn't support the MODE SENSE command
> or it returned useless data.  As a result, we will assume it has a
> write-through cache when it might not.
> 
> For memory sticks this doesn't matter.  For other devices it might be 
> more important (although anything with a working cache should not hit 
> this error case).
> 
> > Of course, an easy way out would be to downgrade both the "Asking for
> > cache data failed" and the "No Caching mode page present" errors to
> > notices. But the SCSI people might disagree with that approach.
> 
> Well, let's see what they say.

What ever happened here, are these 3 patches acceptable, or do they need
to be reworked or something else?

thanks,

greg k-h

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Paul Bolle]

On Tue, 2012-05-15 at 08:35 -0700, Greg Kroah-Hartman wrote:
> > > Of course, an easy way out would be to downgrade both the "Asking for
> > > cache data failed" and the "No Caching mode page present" errors to
> > > notices. But the SCSI people might disagree with that approach.
> > 
> > Well, let's see what they say.
> 
> What ever happened here, are these 3 patches acceptable, or do they need
> to be reworked or something else?

Nothing happened after Alan's message.

So, currently only the first patch was acceptable (to Alan, that is, but
it touched the SCSI code).

At least, I haven't seen a reply from the SCSI people. Perhaps the best
thing to do is to submit a series of one or two trivial patches that
just downgrade the errors involved to notices. That should lead to
feedback by the SCSI people.


Paul Bolle

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Alan Cox]

On Tue, 15 May 2012 18:29:04 +0200
Paul Bolle <pebolle@tiscali.nl> wrote:

> On Tue, 2012-05-15 at 08:35 -0700, Greg Kroah-Hartman wrote:
> > > > Of course, an easy way out would be to downgrade both the "Asking for
> > > > cache data failed" and the "No Caching mode page present" errors to
> > > > notices. But the SCSI people might disagree with that approach.
> > > 
> > > Well, let's see what they say.
> > 
> > What ever happened here, are these 3 patches acceptable, or do they need
> > to be reworked or something else?
> 
> Nothing happened after Alan's message.
> 
> So, currently only the first patch was acceptable (to Alan, that is, but
> it touched the SCSI code).
> 
> At least, I haven't seen a reply from the SCSI people. Perhaps the best
> thing to do is to submit a series of one or two trivial patches that
> just downgrade the errors involved to notices. That should lead to
> feedback by the SCSI people.

Possibly we should have a hostadapter flag indicating if the cache noise
is expected nuisance or not. That way if hardcore SCSI on real cables
wants to keeep it then it can still be shut up for USB where its the
norm and just irrelevant noise.

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Paul Bolle]

On Tue, 2012-05-15 at 17:36 +0100, Alan Cox wrote:
> Possibly we should have a hostadapter flag indicating if the cache noise
> is expected nuisance or not. That way if hardcore SCSI on real cables
> wants to keeep it then it can still be shut up for USB where its the
> norm and just irrelevant noise.

0) Nobody suggested anything else so maybe I should try to come up with
a patch that does that. A slight problem is that I'm not at all familiar
with the SCSI code. And I don't think I own any actual SCSI hardware.

1) Anyhow: hostadapter flags? Are those perhaps the bitfields starting
with "active_mode" in "struct Scsi_Host"?


Paul Bolle

Re: [PATCH 0/3] scsi: usb-storage: hide errors for five devices

[Alan Cox]

On Tue, 22 May 2012 10:48:34 +0200
Paul Bolle <pebolle@tiscali.nl> wrote:

> On Tue, 2012-05-15 at 17:36 +0100, Alan Cox wrote:
> > Possibly we should have a hostadapter flag indicating if the cache noise
> > is expected nuisance or not. That way if hardcore SCSI on real cables
> > wants to keeep it then it can still be shut up for USB where its the
> > norm and just irrelevant noise.
> 
> 0) Nobody suggested anything else so maybe I should try to come up with
> a patch that does that. A slight problem is that I'm not at all familiar
> with the SCSI code. And I don't think I own any actual SCSI hardware.

Give it a go.. only one way to learn 8)

> 1) Anyhow: hostadapter flags? Are those perhaps the bitfields starting
> with "active_mode" in "struct Scsi_Host"?

It can probably go in the scsi_host_template. It's going to be per
adapter and fixed. It will still be accessible then via
host->hostt->whatever. That also makes it easier to set correctly.

Alan