lkml.org 
[lkml]   [2007]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PATCH] USB autosuspend fixes for 2.6.23-rc6
On Thu, Sep 13, 2007 at 03:13:46PM -0400, Alan Stern wrote:
> On Thu, 13 Sep 2007, Linus Torvalds wrote:
>
> > In general, I think the USB blacklist/whitelists are generally a sign of
> > some deeper bug.
> >
> > We used to have a lot of those things due to simply incorrect SCSI
> > probing, causing devices to lock up because Linux probed them with bad or
> > unexpected modepages etc. I suspect we still have old blacklist entries
> > from those days that just never got cleaned up, because nobody ever dared
> > remove the blacklist entry.
>
> I don't just suspect -- I know for a fact that we do. Partly because
> of laziness and partly because of not being able to verify that an
> entry is no longer needed.

We do have some code that looks for unneeded entries. When found by an
end-user (which confirms that the entry can be removed), it asks the user
to drop us an e-mail so we can remove it.

> > We should strive to make the default behaviour be so safe that we never
> > need a black-list (or a whitelist), and basically consider blacklists to
> > be not a way to "fix up a device", but a way to avoid some really serious
> > AND *RARE* error.
>
> In general I agree. However there are some problems for which nobody
> has been able to come up with another solution. See below.

We generally do strive for such a thing. Over the years, we've made
several changes to the way the SCSI core works (especially in the probing
department) to allow us to remove all sorts of special-case code and quirk
entries.

> > For example, why do we have that US_FL_MAX_SECTORS_64 at all? The fact
> > that some USB device is broken with more than 64 sectors would seem to
> > indicate that Windows *never* does more than 64 sectors, and that in turn
> > means that pretty much *no* devices have ever been tested with anything
> > bigger.
> >
> > So why not make the 64 sector limit be the default? Get rid of the quirk:
> > we already allow people to override it in /sys if they really want to, but
> > realistically, it's probably not going to make any difference what-so-ever
> > for *any* normal load. So we seem to have a quirk that really doesn't buy
> > us anything but headache.
>
> That's true now, but it wasn't always. Until the last year or so,
> cdrecord wouldn't work properly with USB CD drives having a 64-sector
> limit unless the user added a particular command-line argument.
>
> In fact, setting max_sectors down to 64 is probably overkill -- 120
> ought to be enough. But there may have been one or two oddball devices
> that really did have a 32-KB limit, and better safe than sorry. At one
> point an engineer from Genesys said their devices did, although they do
> seem to work perfectly well with 64-KB transfers (and that's what
> Windows gives them).

It's worth pointing out that performance drops like a stone as this number
goes down.

> > Other quirks worth looking at (but likely unfixable) are:
> > - US_FL_IGNORE_RESIDUE:
> > Does this really matter? Can we not just always do the
> > US_FL_IGNORE_RESIDUE thing? Windows must not be doing what we're
> > doing.
>
> Windows does indeed ignore the residue field, as far as I can tell.
>
> But this is a rather tricky thing. The USB mass-storage spec
> specifically says that one way a device can signal a short transfer is
> to pad the data with 0s to the requested length and then set the
> residue to indicate how much of the data is valid. If we ignore the
> residue then we run a risk of misinterpreting the 0s as valid data.
>
> Now in practice this doesn't matter much because short transfers of
> block data (READ_10) generally involve other errors that would show up
> anyway, and for non-block data (MODE SENSE) the padding probably
> wouldn't matter. Still it seems like a dangerous sort of thing to do,
> which is why I have resisted it.
>
> (And by the way, there _definitely_ are devices which use this
> signalling method. In fact, Linux contains a driver that does it.)

I think this last point is key. I'm unwilling to sacrifice error detection
on properly working devices to enable error-prone use on clearly buggy
devices.

> > - US_FL_FIX_CAPACITY:
> > This is a generic SCSI issue, not a USB one, and maybe there are
> > better solutions to it. Are we perhaps doing something wrong? Is
> > there some patterns we haven't seen? Why do we need this, when
> > presumably Windows does not?
>
> This is another hard case. No, we aren't doing anything wrong. If
> there are any patterns we haven't seen, we aren't aware of them. :-)
> You might think that if a device claims to have an odd number of
> sectors then it must be wrong, but this turns out not to be true.
>
> Why doesn't Windows need this? For all we know, it does. Has anybody
> ever tried forcing Windows to read the sector beyond the end of one of
> these buggy devices?

As far as I know, Windows doesn't need this because of the way FAT and NTFS
work. They never use the end of the disk (by more than a few sectors, or
so I'm told).

> There's a straightforward solution: Never try to use the last sector --
> in effect, assume every device has the FIX_CAPACITY flag set. Doing
> this universally could cause data loss, however, so again I have been
> opposed to it.

I agree here.

> > - US_FL_SINGLE_LUN:
> > At least a few of these seem to indicate that the real problem
> > could be detected dynamically ("device reports Sub=ff") rather
> > than with a quirk. Quirks are unmaintainable (and change), but
> > noticing when devices return impossible values and going into a
> > "safe mode" is just defensive programming.
>
> This is almost certainly a case where lots of the entries are no longer
> needed. But it isn't easy to tell which ones can safely be removed.

I've been meaning to start sending e-mails to see if we can get rid of
these. Most of the devices which required it were UFI, which reports "LUN
not present" in a goofy way. We fixed the code to detect it properly, but
there are still quite a few devices out there that don't implement the
correct (if goofy) method.

Most of those entries (which are for UFI devices) can go, if we get a
volunteer to take the e-mail addresses listed in unusual_devs.h and work
the list.

Matt

--
Matthew Dharm Home: mdharm-usb@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver

Sir, for the hundreth time, we do NOT carry 600-round boxes of belt-fed
suction darts!
-- Salesperson to Greg
User Friendly, 12/30/1997
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2007-09-14 02:43    [W:0.064 / U:7.152 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site