stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <jbottomley@parallels.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Ben Hutchings <ben@decadent.org.uk>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"alan@lxorguk.ukuu.org.uk" <alan@lxorguk.ukuu.org.uk>,
	Alan Stern <stern@rowland.harvard.edu>,
	Matthew Dharm <mdharm-usb@one-eyed-alien.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-scsi <linux-scsi@vger.kernel.org>
Subject: Re: [ 38/48] SCSI & usb-storage: add try_rc_10_first flag
Date: Mon, 2 Jul 2012 20:39:28 +0000	[thread overview]
Message-ID: <1341261568.3319.7.camel@dabdike> (raw)
In-Reply-To: <CA+55aFyRTXqSvf9m08gbHCB0WhVF14Y6J2-AiFGiuTStq0Jr6Q@mail.gmail.com>

On Mon, 2012-07-02 at 11:52 -0700, Linus Torvalds wrote:
> On Mon, Jul 2, 2012 at 12:10 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > So since 3.2 already tries READ_CAPACITY(10) first, this patch is
> > not needed. It does not harm either, and if the scsi level reporting
> > change ever finds its way into 3.2 stable (which it should not
> > IMHO), then this patch will be needed.
> 
> Yeah, it should definitely not make it into stable. In fact, I'd
> rather remove it from mainline too. The whole idea to do
> READ_CAPACITY_16 by default was moronic.
> 
> James? Why does the SCSI subsystem always do these idiotic defaults?
> We *know* that devices are buggy. Sure, USB storage tends to be
> buggier than most, but the whole idea of "let's use the new way of
> reporting that gets almost no testing" is just wrong.

It's not idiotic.  It's, unfortunately, required by the standard to
check for protection capable devices.  It's been this way for three
years or so.  Martin Petersen did extensive testing of devices when we
changed it and doing RC16 first is hedged around by claiming support not
only for SCSI_3 but also for SBC_2 in your INQUIRY data.

The problem seems to be that when all the testing was done, USB was
still always returning SCSI_2 in the inquiry data, but this got changed
to allow pass through of the actual vendor information.

> I don't think the USB layer should have needed to work around this. I
> don't think there should be a "try_10b" flag at all. I think the SCSI
> code should just *never* do the 16b version unless it has done the 10b
> version first and determined that the size doesn't fit.

If we did that, we'd regress on all the enterprise class drives with DIF
and DIX.

> And the code already had that logic got the read_10b case. The whole
> logic to do 16b by default was crap. It *used* to at least only do it
> for disks that were marked as having "protection", now it wants to do
> it for anything that should be able to handle it. Which is bogus. The
> whole "let's assume devices get it right" mindset is broken.
> 
> The mindset should be "let's assume devices are buggy, and be careful".

It still is, that's why we did extensive testing three years ago before
changing it.

If you have a solution that doesn't break all the enterprise devices but
works better for USB, I'm listening.  I think the best thing might be to
whitelist the scsi revision pass through in USB, so only the UAS devices
which really, really claim to know what they're doing advertise advanced
features.

James


  reply	other threads:[~2012-07-02 20:39 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20120701172006.535271340@decadent.org.uk>
2012-07-01 17:20 ` [ 01/48] staging:iio:ad7606: Re-add missing scale attribute Ben Hutchings
2012-07-01 17:20 ` [ 02/48] Tools: hv: verify origin of netlink connector message Ben Hutchings
2012-07-01 17:20 ` [ 03/48] edac: avoid mce decoding crash after edac driver unloaded Ben Hutchings
2012-07-01 17:20 ` [ 06/48] hwmon: (applesmc) Limit key length in warning messages Ben Hutchings
2012-07-01 17:20 ` [ 07/48] mm: fix slab->page _count corruption when using slub Ben Hutchings
2012-07-02 23:46   ` Herton Ronaldo Krzesinski
2012-07-02 23:56     ` Herton Ronaldo Krzesinski
2012-07-03  1:17       ` Herton Ronaldo Krzesinski
2012-07-03 20:19         ` Pravin Shelar
2012-07-04  4:36     ` Ben Hutchings
2012-07-01 17:20 ` [ 08/48] mm: pmd_read_atomic: fix 32bit PAE pmd walk vs pmd_populate SMP race condition Ben Hutchings
2012-07-01 17:20 ` [ 10/48] nilfs2: ensure proper cache clearing for gc-inodes Ben Hutchings
2012-07-01 17:20 ` [ 12/48] drm/i915: Finish any pending operations on the framebuffer before disabling Ben Hutchings
2012-07-01 17:20 ` [ 13/48] drm/i915: Remove use of the autoreported ringbuffer HEAD position Ben Hutchings
2012-07-01 17:20 ` [ 15/48] e1000e: Remove special case for 82573/82574 ASPM L1 disablement Ben Hutchings
2012-07-01 22:59   ` Jonathan Nieder
2012-07-01 17:20 ` [ 16/48] drm/i915: Do the fallback non-IRQ wait in ring throttle, too Ben Hutchings
2012-07-01 17:20 ` [ 17/48] staging:rts_pstor:Fix possible panic by NULL pointer dereference Ben Hutchings
2012-07-01 17:20 ` [ 21/48] drm/nouveau/fbcon: using nv_two_heads is not a good idea Ben Hutchings
2012-07-01 17:20 ` [ 29/48] ARM: SAMSUNG: Should check for IS_ERR(clk) instead of NULL Ben Hutchings
2012-07-01 17:20 ` [ 30/48] ath9k_hw: avoid possible infinite loop in ar9003_get_pll_sqsum_dvc Ben Hutchings
2012-07-01 17:20 ` [ 31/48] iwlwifi: remove log_event debugfs file debugging is disabled Ben Hutchings
2012-07-01 17:20 ` [ 32/48] ARM: SAMSUNG: Fix for S3C2412 EBI memory mapping Ben Hutchings
2012-07-01 17:20 ` [ 33/48] USB: option: add id for Cellient MEN-200 Ben Hutchings
2012-07-01 17:20 ` [ 34/48] oprofile: perf: use NR_CPUS instead or nr_cpumask_bits for static array Ben Hutchings
2012-07-01 17:20 ` [ 35/48] drm/i915: Refactor the deferred PM_IIR handling into a single function Ben Hutchings
2012-07-01 17:20 ` [ 36/48] drm/i915: rip out the PM_IIR WARN Ben Hutchings
2012-07-01 17:20 ` [ 37/48] drm/i915: Fix eDP blank screen after S3 resume on HP desktops Ben Hutchings
2012-07-01 17:20 ` [ 38/48] SCSI & usb-storage: add try_rc_10_first flag Ben Hutchings
2012-07-02  7:10   ` Hans de Goede
2012-07-02 18:52     ` Linus Torvalds
2012-07-02 20:39       ` James Bottomley [this message]
2012-07-02 22:23         ` Linus Torvalds
2012-07-03  0:41           ` Matthew Wilcox
2012-07-03  6:18             ` James Bottomley
2012-07-03 15:49               ` Alan Stern
2012-07-03 17:32                 ` Matthew Wilcox
2012-07-03 19:50                   ` Alan Stern
2012-07-03 20:07                     ` James Bottomley
2012-07-03 20:25                       ` Alan Stern
2012-07-03 20:35                       ` Matthew Wilcox
2012-07-05 21:40                         ` Alan Stern
2012-07-06  3:05                           ` Matthew Wilcox
2012-07-06 14:00                             ` Alan Stern
2012-07-04  4:39     ` Ben Hutchings
2012-07-01 17:20 ` [ 39/48] PM / Sleep: Prevent waiting forever on asynchronous suspend after abort Ben Hutchings
2012-07-01 17:20 ` [ 40/48] x86, cpufeature: Rename X86_FEATURE_DTS to X86_FEATURE_DTHERM Ben Hutchings
2012-07-01 17:20 ` [ 42/48] ALSA: hda - Add Realtek ALC280 codec support Ben Hutchings
2012-07-01 17:20 ` [ 43/48] USB: option: Add USB ID for Novatel Ovation MC551 Ben Hutchings
2012-07-01 17:20 ` [ 45/48] xen/netfront: teardown the device before unregistering it Ben Hutchings
2012-07-01 17:20 ` [ 46/48] can: flexcan: use be32_to_cpup to handle the value of dt entry Ben Hutchings
2012-07-01 17:20 ` [ 47/48] acpi_pad: fix power_saving thread deadlock Ben Hutchings
2012-07-01 17:20 ` [ 48/48] batman-adv: only drop packets of known wifi clients Ben Hutchings
2012-07-01 19:11 ` [ 00/48] 3.2.22-stable review Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1341261568.3319.7.camel@dabdike \
    --to=jbottomley@parallels.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=ben@decadent.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdegoede@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=mdharm-usb@one-eyed-alien.net \
    --cc=stable@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).