[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1317406267.2993.33.camel@dabdike.hansenpartnership.com>
Date: Fri, 30 Sep 2011 13:11:07 -0500
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Amit Sahrawat <amit.sahrawat83@...il.com>
Cc: linux-usb@...r.kernel.org, linux-scsi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Christoph Hellwig <hch@...radead.org>
Subject: Re: BUG in kernel: Wrong Handling of USB HDD’s
in scsiglue(slave_configure) and scsi/sd(sd_read_cache_type)
On Fri, 2011-09-30 at 23:26 +0530, Amit Sahrawat wrote:
> Adding linux-usb - to get more insight's into the problem.
>
> On Fri, Sep 30, 2011 at 11:23 PM, Amit Sahrawat
> <amit.sahrawat83@...il.com> wrote:
> > On Fri, Sep 30, 2011 at 5:48 PM, James Bottomley
> > <James.Bottomley@...senpartnership.com> wrote:
> >> On Fri, 2011-09-30 at 12:26 +0530, Amit Sahrawat wrote:
> >>> Now, for the USB HDD which do have write cache - sginfo is showing
> >>> them to Write Cache Enabled as false.
> >>> Why do the result of hdparm identification and sginfo varies- (I know
> >>> they have different interface to work with and hdparm takes care of
> >>> that by using SG_IO interface from it's code)? hdparm showed me
> >>> correct results - that lead me to digging in the kernel code and
> >>> checking the performance for USB HDD with Write cache enabled/disabled
> >>> - which also showed that QUEUE ordering chosen for USB HDD is not
> >>> correct.
> >>
> >> Well, what all this means is the SATL in the USB device is implemented
> >> wrongly. Since USB devices only preset SCSI interfaces, that's what we
> >> have to believe.
> >>
> >> hdparm when used correctly sends an ATA inquiry command wrapped in an
> >> ATA_12 or ATA_16 SCSI command. A large number of legacy SATLs are known
> >> to crash on these commands.
> >>
> >> Are you sure the ATA command is reporting correctly? A write back cache
> >> is a remarkably silly thing to enable for a USB device because they're
> >> highly likely to be surprise ejected which powers the device down.
> >>
> > In addition to the problem reported - there is one more thing I have
> > noticed with USB HDD - they should be shown as 'removable' but the
> > removable is marked only for USB PEN Drives. This seems to be a bit of
> > confusing, any mass storage media connected on USB port should be
> > recognized as removable.
I don't really think so. Removable to sd means that the drive can be
removed from the housing, not that the connector cable is hot plug so
the whole disk can disappear. I think your disk is the latter, and
therefore removable probably isn't what you want (otherwise the sd
driver will start probing for medium change and other things your device
won't understand).
> > So, for handling the issue, I would consider adding the handling in
> > slave_configure()(usb/storage/scsiglue) which marks the HDD/pen drives
> > as removable also signifying them to be USB based.
> > Then, as part of sd_revalidation – how about adding the ATA_IDENTIFY
> > command part if the device is USB HDD? As far as the result of
> > ATA_IDENTIFY is concerned – they return proper ‘256’ bytes - response
> > and the Words – 82, 85 used for feature supported and enabled/disabled
> > returns proper values for the USB HDD’s I have seen. In case of USB
> > pen drives – they return failure – I did not see any crash – maybe I
> > don’t have one of the legacy SATL based disk.
> > Since, I am new to this – I will check more on this to get a viable
> > solution. Please add your opinion. Can you share the name of the
> > device which causes crash with these ATA commands, If I am able to get
> > one I can try on that also.
How? There are many USB devices whose SATL crashes and burns on ATA_12
or ATA_16, so this would add fragility to the detection path. The
detection path has to be cast iron because it has to work on a vast
range of devices. The only way this could really work is to add it in
the usb storage driver and then replace the bad mode pages.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists