Re: JMS56x not working reliably with uas driver
On Wed, 2016-12-21 at 17:09 +0530, George Cherian wrote: > Hi Oliver, > > I was working with this JMicron device and using the uas driver. > I am seeing the following 2 issues. > > 1) On connect I see the following messages. Thanks. Do you want to submit it to Greg? The patch is fine. > 2) On disconnect I am seeing the following issue > > scsi host4: uas_post_reset: alloc streams error -19 after reset > sd 4:0:0:0: [sdb] Synchronizing SCSI cache > > This is more fatal because after these messages the USB port becomes > unusable. Even an lsusb invocation hangs for ever. Ouch. That points to a logic error. We should not reset if a device is gone. Could you send dmesg of such a case? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Wed, 2016-12-21 at 12:54 +0100, Hans de Goede wrote: > Hi, > > On 21-12-16 12:42, Oliver Neukum wrote: > > On Wed, 2016-12-21 at 17:09 +0530, George Cherian wrote: > >> Hi Oliver, > >> > >> I was working with this JMicron device and using the uas driver. > >> I am seeing the following 2 issues. > >> > >> 1) On connect I see the following messages. > > > > Thanks. Do you want to submit it to Greg? > > The patch is fine. > > > >> 2) On disconnect I am seeing the following issue > >> > >> scsi host4: uas_post_reset: alloc streams error -19 after reset > >> sd 4:0:0:0: [sdb] Synchronizing SCSI cache > >> > >> This is more fatal because after these messages the USB port becomes > >> unusable. Even an lsusb invocation hangs for ever. > > > > Ouch. That points to a logic error. We should not reset if > > a device is gone. > > Ah good point, so instead of just seeing a disconnect, something > is triggering a reset, and then we try to alloc stream on the > disconnected usb port and that apparently makes the xhci controller > quite unhappy. It is problematic because there is a window we cannot avoid. This needs a full dmesg to say anything more about it. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Wed, 2016-12-21 at 17:37 +0530, George Cherian wrote: > > On 12/21/2016 05:12 PM, Oliver Neukum wrote: > > On Wed, 2016-12-21 at 17:09 +0530, George Cherian wrote: > >> Hi Oliver, > >> > >> I was working with this JMicron device and using the uas driver. > >> I am seeing the following 2 issues. > >> > >> 1) On connect I see the following messages. > > > > Thanks. Do you want to submit it to Greg? > > The patch is fine. > Yes please!!! So you want me to submit it? It would be your chance to get a patch upstream. > >> 2) On disconnect I am seeing the following issue > >> > >>scsi host4: uas_post_reset: alloc streams error -19 after reset > >>sd 4:0:0:0: [sdb] Synchronizing SCSI cache > >> > >> This is more fatal because after these messages the USB port becomes > >> unusable. Even an lsusb invocation hangs for ever. > > > > Ouch. That points to a logic error. We should not reset if > > a device is gone. > > Could you send dmesg of such a case? > here is the dmesg!! > [ 203.475382] usb 4-1.3: new SuperSpeed USB device number 3 using xhci_hcd > [ 203.496172] usb 4-1.3: New USB device found, idVendor=152d, > idProduct=9561 > [ 203.503037] usb 4-1.3: New USB device strings: Mfr=1, Product=2, > SerialNumber=5 > [ 203.510352] usb 4-1.3: Product: JMS56x Series > [ 203.514698] usb 4-1.3: Manufacturer: JMicron > [ 203.518966] usb 4-1.3: SerialNumber: > [ 203.594383] usbcore: registered new interface driver usb-storage > [ 203.612425] scsi host4: uas > [ 203.615418] usbcore: registered new interface driver uas > [ 203.620979] scsi 4:0:0:0: Direct-Access ST4000NM 0033-9ZM170 > 0001 PQ: 0 ANSI: 6 > [ 203.630240] sd 4:0:0:0: Attached scsi generic sg1 type 0 > [ 203.630382] sd 4:0:0:0: [sdb] 7814037168 512-byte logical blocks: > (4.00 TB/3.63 TiB) > [ 203.631338] sd 4:0:0:0: [sdb] Write Protect is off > [ 203.631342] sd 4:0:0:0: [sdb] Mode Sense: 67 00 10 08 > [ 203.631734] sd 4:0:0:0: [sdb] Write cache: enabled, read cache: > enabled, supports DPO and FUA > [ 203.631899] xhci_hcd :00:11.0: ERROR Transfer event for disabled > endpoint or incorrect stream ring > [ 203.631904] xhci_hcd :00:11.0: @001f610a1c10 > 1b00 03078001 state 14 ep_info 9403 > [ 203.631906] xhci_hcd :00:11.0: No epring > [ 203.674546] sdb: sdb1 > [ 203.676639] sd 4:0:0:0: [sdb] Attached SCSI disk > [ 213.222913] scsi host4: uas_post_reset: alloc streams error -19 after > reset > [ 213.230548] sd 4:0:0:0: [sdb] Synchronizing SCSI cache > > Above is the dmesg without the unusual_uas patch applied. > Do you need me to enable any specific dev_dbg and then the dmesg output? Damn, that is a strange error. Do you know whether it happens on other XHCI controllers? Which controller are you using and can you please also post "lsusb -v"? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Wed, 2016-12-21 at 18:17 +0530, George Cherian wrote:
> [ 843.149653] scsi host5: uas_post_reset: alloc streams error -19
> after
> reset
That would mean the endpoints are gone. Which is odd.
> [ 843.157268] sd 5:0:0:0: [sdb] Synchronizing SCSI cache
Could you try the attached patch and do a SCSI log of the enumeration?
Regards
Oliver
From d4ddac88bbf9cb15e7d8638582f96d31e245f15b Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Wed, 21 Dec 2016 15:34:54 +0100
Subject: [PATCH] uas: device crashes on reset
We avoid resetting it.
Signed-off-by: Oliver Neukum
---
drivers/usb/core/quirks.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index d2e50a2..52483fb 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -194,6 +194,8 @@ static const struct usb_device_id usb_quirk_list[] = {
{ USB_DEVICE(0x1532, 0x0116), .driver_info =
USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL },
+ { USB_DEVICE(0x152d, 0x9561), .driver_info = USB_QUIRK_RESET },
+
/* BUILDWIN Photo Frame */
{ USB_DEVICE(0x1908, 0x1315), .driver_info =
USB_QUIRK_HONOR_BNUMINTERFACES },
--
2.1.4
Re: JMS56x not working reliably with uas driver
On Thu, 2016-12-22 at 15:43 +0530, George Cherian wrote: > dmesg with the patch applied Hi, did you apply both patches, yours and the one I posted? And did you physically disconnect your device? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Thu, 2016-12-22 at 17:44 -0500, Alan Stern wrote: > I don't see how this patch fixes anything. Unless I'm mistaken, it > just avoids the problem by preventing the system from issuing the > command that provokes the error, rather than really fixing the > underlying error. Please clarify. If a reset leads to a disconnect, isn't that exactly what we want? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Tue, 2016-12-27 at 10:20 -0500, Alan Stern wrote: > On Tue, 27 Dec 2016, Oliver Neukum wrote: > > > On Thu, 2016-12-22 at 17:44 -0500, Alan Stern wrote: > > > I don't see how this patch fixes anything. Unless I'm mistaken, it > > > just avoids the problem by preventing the system from issuing the > > > command that provokes the error, rather than really fixing the > > > underlying error. > > > > Please clarify. If a reset leads to a disconnect, isn't that > > exactly what we want? > > I didn't express myself clearly enough. Yes, if a reset leads to a > disconnect then avoiding the reset will avoid problems. Good. Then we need to clarify whether the device was physically disconnected when the logs were taken. > But the _real_ error here is that xhci-hcd says "ERROR Transfer event > for disabled endpoint or incorrect stream ring" when the disconnect > occurs during reset. That shouldn't happen, no matter what quirks the > device has. It indicates a bug either in uas or in xhci-hcd. True. I am afraid that there necessarily is a window for resetting a disconnected device. But the check you proposed is better. however, I'd like to encapsulate that together with a test for logical disconnect. Uas is unlikely to be the only driver that has this issue. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: JMS56x not working reliably with uas driver
On Tue, 2016-12-27 at 21:19 -0500, Alan Stern wrote: > On Tue, 27 Dec 2016, George Cherian wrote: > > True. I am afraid that there necessarily is a window for resetting a > > disconnected device. But the check you proposed is better. > > however, I'd like to encapsulate that together with a test for > > logical disconnect. Uas is unlikely to be the only driver that has > > this issue. > > True enough. We could have a usb_device_is_disconnected() inline > helper for this purpose. There's no need to try to distinguish > between physical and logical disconnects, as far as I can see. There is no such need. There is a need for both checks though. > However, as George points out, the "Transfer event" error has nothing > to do with disconnection. It was triggered by the device's bogus > response to a REPORT OPCODES command, or something of the sort. We > should be able to handle bogus responses without logging ERROR > messages, because such messages indicate a bug in a kernel driver. Indeed. We have two independent issues. We should have two independent fixes. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: AS2105-based enclosure size issues with >2TB HDDs
On Mon, 2014-08-25 at 10:58 +, Alfredo Dal Ava Junior wrote:
> - 1TB and 2TB: READ_CAPACITY_10 returns correct size value
> - 3TB and 4TB: READ_CAPACITY_10 returns size in a 2TB modulus
>
> If we fix capacity size by reporting (READ_CAPACITY_10 + MODULO_2TB), the
> result
> will be invalid when user plugs a <2TB HDD. An idea (bring by Oliver) is:
It is worse than that. Pretty soon a 4.7TB disk will also be plausible.
> first guess reading last sector using modulus result to check if size is
> valid.
It is necessary that a virgin disk also be handled correctly.
We cannot use the partition table (besides it being a layering
violation)
It would propose (in pseudo code)
if (read_capacity_16(device) < 0) {
lower_limit = read_capacity_10(device);
for (i = 1; i++; i < SANITY_LIMIT) {
err = read_sector(device, lower_limit + i * 2TB-SAFETY);
if (err == OUT_OF_RANGE)
break;
}
if (i < SANITY_LIMIT)
return (i - 1) * 2TB + lower_limit;
else
return ERROR;
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Mon, 2014-08-25 at 16:21 -0400, Alan Stern wrote: > On Mon, 25 Aug 2014, Alfredo Dal Ava Junior wrote: > > > Well, it is causing problems anyway... from user perspective, it's a > > Linux compatibility issue, as it works "fine" on Windows. I'm not an > > expert, but I'm wondering that if usb-storage could set capacity as > > "UNDETERMINED"/ Zero (or keep using the readcapacity_10 as it as > with > > some flag signalizing it as inaccurate), EFI partition check would > be > > able to ignore size and look for secondary GPT where it really is. > > Part of the problem is that usb-storage has no way to know that > anything strange is going on. It's normal for READ CAPACITY(16) to > fail (this depend on the SCSI level), and it's normal for the READ > CAPACITY(10) to report a value less than 2 TB. Just set US_FL_NEEDS_CAP16. If READ CAPACITY(16) fails in that case, it is clear that something is wrong. It must be set or READ CAPACITY(10) alone would be taken as giving a valid answer. At that time we are sure that the drive will be unusable unless we determine the correct capacity, so we don't make matters worse if we crash it. Is there an easy way for Alfredo to determine what happens if we read beyond the end? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Tue, 2014-08-26 at 09:58 +, David Laight wrote: > > Part of the problem is that usb-storage has no way to know that > > anything strange is going on. It's normal for READ CAPACITY(16) to > > fail (this depend on the SCSI level), and it's normal for the READ > > CAPACITY(10) to report a value less than 2 TB. > > Could the code try READ CAPACITY(16) first? Yes. It does already. That fails as the device doesn't support this version. In a way we are discussing error handling here. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RES: RES: AS2105-based enclosure size issues with >2TB HDDs
On Tue, 2014-08-26 at 10:47 -0400, Alan Stern wrote: > On Mon, 25 Aug 2014, Oliver Neukum wrote: > > Just set US_FL_NEEDS_CAP16. If READ CAPACITY(16) fails in that case, > > it is clear that something is wrong. It must be set or READ CAPACITY(10) > > alone would be taken as giving a valid answer. > > You have committed a mental layering violation. :-) Indeed. > US_FL_NEEDS_CAP16 is a usb-storage flag. But the capacity > determination is made by the sd driver, which relies on the SCSI > try_rc_10_first flag. If that flag isn't set, sd tries READ > CAPACITY(16) and then falls back to READ CAPACITY(10) if an error > occurs. That's what happened here. > > I don't think we want to add another SCSI flag to say that READ > CAPACITY(10) is unreliable. Why not? It would only be friendly to tell the upper layer of a malfunction if we know about it. > > At that time we are sure that the drive will be unusable unless we > > determine the correct capacity, so we don't make matters worse if we > > crash it. > > Given the difficulty of determining the true capacity, I see two > alternatives. We could set the capacity to a ridiculously large value > (like 1 billion TB), or we could leave the capacity set to the low > value and disable the "block within bounds" checks. Neither of these > is attractive and they both have issues of their own -- although the > second is close to what Windows does. That seems to be the most attractive solution to me. > (For example, udev often tries to read the last sectors of a new drive, > looking for a GPT or RAID signature. That won't work if the capacity > is set to some random value.) Yes, but clipping has its own dangers. Suppose you use the medium without a partition table. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/21] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
On Wed, 2014-09-10 at 13:48 +0200, Hans de Goede wrote: > Hi, > > Note this series is NOT intended for stable, but I accidentally > had "cc = sta...@vger.kernel.org" in my .git/config when sending > this series, please ignore for stable. > > NACK for stable. If this is not for stable, what do you intend to do about the problems in stable? For example patch#01 of this series looks like clear stable material to me. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/21] uas: replace WARN_ON_ONCE() with lockdep_assert_held()
On Wed, 2014-09-10 at 14:00 +0200, Hans de Goede wrote: > Hi, > > On 09/10/2014 01:56 PM, Oliver Neukum wrote: > > On Wed, 2014-09-10 at 13:48 +0200, Hans de Goede wrote: > >> Hi, > >> > >> Note this series is NOT intended for stable, but I accidentally > >> had "cc = sta...@vger.kernel.org" in my .git/config when sending > >> this series, please ignore for stable. > >> > >> NACK for stable. > > > > If this is not for stable, what do you intend to do about > > the problems in stable? For example patch#01 of this series > > looks like clear stable material to me. > > The plan for stable is mostly, as lame as that is, to make sure > we get all the right quirks in place so that error handling > does not get triggered, for now. How? A medium can be defect. Short of entirely disabling it, error handling will be triggered. > I agree that once this set has seen wider testing, we should > reconsider, and probably add it, to stable. But at this point > in time I'm worried that it may cause regressions, and as such > it is not stable material atm IHMO. Well, we would exchange something known to work imperfectly for something feared to work imperfectly. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/21] uas: Remove task-management / abort error handling code
On Wed, 2014-09-10 at 13:46 +0200, Hans de Goede wrote: > This commit removes the abort / lun-reset error handling paths, and also the > taks-mgmt code since those are the only 2 task-mgmt users. Leaving only the > (tested and testable) usb-device-reset error handling path in place. > > Note I realize that this is somewhat of a big hammer, but currently people > are seeing very hard to debug oopses with uas. First let focus on making uas > work reliable, then we can later look into adding more fine grained error > handling. This hurts. But I suppose it is the conservative choice. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/21] uas: Fix resetting flag handling
On Wed, 2014-09-10 at 13:46 +0200, Hans de Goede wrote: > - Make sure we always hold the lock when setting / checking resetting > - Check resetting before checking urb->status > - Add missing check for resetting to uas_data_cmplt > - Add missing check for resetting to uas_do_work Why is the checking for stat and data inconsistent? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/21] uas: Do not log urb status error on cancellation
On Wed, 2014-09-10 at 13:46 +0200, Hans de Goede wrote: > Check for both type of cancellation codes for sense and data urbs. Then you should also check for -ESHUTDOWN (unplug of HC) Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 20/21] uas: Remove support for old sense ui as used in pre-production hardware
On Wed, 2014-09-10 at 13:46 +0200, Hans de Goede wrote: > I've access to a number of different uas devices now, and none of them use > old style sense urbs. The only case where these code-paths trigger is with > the asm1051 and there they do the wrong thing, as the asm1051 sends 8 bytes > status iu-s when it does not have any sense data, but uses new style > sense iu-s regardless, as can be seen for scsi cmnds where there is sense > data. This looks like a bad idea over all. The version is in the spec. And you might see true USB<->SCSI bridges passing through the sense data. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/21] uas: zap_pending: data urbs should have completed at this time
On Wed, 2014-09-10 at 13:46 +0200, Hans de Goede wrote: > uas_log_cmd_state(cmnd, __func__); > - /* all urbs are killed, clear inflight bits */ > - cmdinfo->state &= ~(COMMAND_INFLIGHT | > - DATA_IN_URB_INFLIGHT | > - DATA_OUT_URB_INFLIGHT); > + /* Sense urbs were killed, clear COMMAND_INFLIGHT > manually */ > + cmdinfo->state &= ~COMMAND_INFLIGHT; > cmnd->result = result << 16; > - uas_try_complete(cmnd, __func__); > + WARN_ON(uas_try_complete(cmnd, __func__) != 0); That looks like very bad style. WARN_ON() shouldn't have side effects. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH fix for 3.17] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands
On Mon, 2014-09-15 at 08:42 +, David Laight wrote: > From: Alan Stern > > > You must not add an aditional value for a module parameter without > > documenting it in Documentation/kernel-parameters.txt. > > How can this work as a 'module parameter'? It cannot. This parameter is an aid to debugging. > I might want to use two different usb-scsi devices that have different > requirements. Indeed. Quirky devices must be added to the quirks file. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/24] uas: zap_pending: data urbs should have completed at this time
On Sun, 2014-09-14 at 11:34 +0100, James Bottomley wrote: > I'm agnostic on that. I was just combatting the impression that you had > to be careful about side effects in known macro statements. No you haven't. But it is very good practice to not have side effects in warnings, as people, not compilers, like to remove them. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Fri, 2013-07-26 at 09:54 -0700, Andy Lutomirski wrote: > This is kernel 3.9.9-302.fc19.x86_64. > > I plugged in a BN Nook (a usb mass storage device), used it, and > ejected it. This makes suspend fail: > > [50135.265514] PM: Entering freeze sleep > [50135.265517] Suspending console(s) (use no_console_suspend to debug) > [50135.287724] sd 7:0:0:0: [sdb] Synchronizing SCSI cache > [50135.290413] sd 7:0:0:0: [sdb] > [50135.290415] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [50135.290418] sd 7:0:0:0: [sdb] > [50135.290422] Sense Key : Not Ready [current] > [50135.290424] sd 7:0:0:0: [sdb] > [50135.290429] Add. Sense: Medium not present > [50135.290448] dpm_run_callback(): scsi_bus_suspend+0x0/0x40 returns -5 > [50135.290454] PM: Device 7:0:0:0 failed to suspend async: error -5 > [50138.486917] PM: Some devices failed to suspend > [50138.525007] PM: resume of devices complete after 38.132 msecs > [50138.525315] pci_pm_runtime_suspend(): > hcd_pci_runtime_suspend+0x0/0x50 returns -16 > [50138.536357] PM: Finishing wakeup. > > Experimenting a bit, here's what happens. > > - Plug in device (so it's working). Suspend works. > - eject /dev/sdb. Suspend fails. > - Physically unplug the device. Suspend works again. Hi, this looks like a logical error in sd_suspend(). It should ignore errors due to missing media (or offlined devices) Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Fri, 2013-07-26 at 16:31 -0400, Alan Stern wrote: > In addition to my earlier comment, the patch below should be applied. > It will fix your immediate problem, although not in the best way. Alan, I think your diagnosis is correct, but not the fix. This is run even in the runtime case. We might lose data if the flush is not done. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Mon, 2013-07-29 at 10:21 -0400, Alan Stern wrote: > On Mon, 29 Jul 2013, Oliver Neukum wrote: > > > On Fri, 2013-07-26 at 16:31 -0400, Alan Stern wrote: > > > > > In addition to my earlier comment, the patch below should be applied. > > > It will fix your immediate problem, although not in the best way. > > > > Alan, > > > > I think your diagnosis is correct, but not the fix. > > This is run even in the runtime case. We might lose > > data if the flush is not done. > > Actually no, the scsi_bus_suspend_common() routine does not run during > runtime PM. It gets called only from scsi_bus_suspend(), > scsi_bus_freeze(), and scsi_bus_poweroff(), which are all part of > system sleep. Well yes, but you handled only the system case that way. The runtime case is still affected. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Tue, 2013-07-30 at 13:00 -0400, Alan Stern wrote: > That's why I said the patch would fix the immediate problem but it > wasn't the best solution. You do agree that the patch is correct, as > far as it goes? It will allow the system to sleep. But it seems to me that a genuine error while flushing a drive's cache is one of the few things actual worth aborting a system suspend for. > If you would like to handle the problems in sd.c concerning the > sync-cache and spin-down stuff, that will be fine with me. Yes. The problem is not limited to USB. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Mon, 2013-07-29 at 10:21 -0400, Alan Stern wrote:
> On Mon, 29 Jul 2013, Oliver Neukum wrote:
>
> > On Fri, 2013-07-26 at 16:31 -0400, Alan Stern wrote:
> >
> > > In addition to my earlier comment, the patch below should be applied.
> > > It will fix your immediate problem, although not in the best way.
> >
> > Alan,
> >
> > I think your diagnosis is correct, but not the fix.
> > This is run even in the runtime case. We might lose
> > data if the flush is not done.
>
> Actually no, the scsi_bus_suspend_common() routine does not run during
> runtime PM. It gets called only from scsi_bus_suspend(),
> scsi_bus_freeze(), and scsi_bus_poweroff(), which are all part of
> system sleep.
These errors should be handled cleanly. How about this patch?
Regards
Oliver
>From 76a377d9894dc8945e9afecc7f9864e6dc3156b1 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Wed, 31 Jul 2013 13:32:51 +0200
Subject: [PATCH] sd: handle errors during suspend
Errors during suspend must be handled according to reasons
Errors due to missing media or unplugged devices must be ignored.
The error returns must be modified so that the generic layer
understands them.
Signed-off-by: Oliver Neukum
---
drivers/scsi/scsi_pm.c | 3 ++-
drivers/scsi/sd.c | 61 +-
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 4c5aabe..af4c050 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -54,7 +54,8 @@ scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
/*
* All the high-level SCSI drivers that implement runtime
* PM treat runtime suspend, system suspend, and system
- * hibernate identically.
+ * hibernate nearly identically. In all cases the requirements
+ * for runtime suspension are stricter.
*/
if (pm_runtime_suspended(dev))
return 0;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 86fcf2c..76273f4 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -105,7 +105,8 @@ static void sd_unlock_native_capacity(struct gendisk *disk);
static int sd_probe(struct device *);
static int sd_remove(struct device *);
static void sd_shutdown(struct device *);
-static int sd_suspend(struct device *);
+static int sd_suspend_system(struct device *);
+static int sd_suspend_runtime(struct device *);
static int sd_resume(struct device *);
static void sd_rescan(struct device *);
static int sd_done(struct scsi_cmnd *);
@@ -483,11 +484,11 @@ static struct class sd_disk_class = {
};
static const struct dev_pm_ops sd_pm_ops = {
- .suspend = sd_suspend,
+ .suspend = sd_suspend_system,
.resume = sd_resume,
- .poweroff = sd_suspend,
+ .poweroff = sd_suspend_system,
.restore = sd_resume,
- .runtime_suspend = sd_suspend,
+ .runtime_suspend = sd_suspend_runtime,
.runtime_resume = sd_resume,
};
@@ -1455,12 +1456,31 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
if (res) {
sd_print_result(sdkp, res);
- if (driver_byte(res) & DRIVER_SENSE)
+
+ if (driver_byte(res) & DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
+ /* we need to evaluate the error return */
+ if ((scsi_sense_valid(&sshdr) &&
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a))
+/* this is no error here */
+return 0;
+
+ switch (host_byte(res)) {
+ /* ignore errors due to racing a disconnection */
+ case DID_BAD_TARGET:
+ case DID_NO_CONNECT:
+ return 0;
+ /* signal the upper layer it might try again */
+ case DID_BUS_BUSY:
+ case DID_IMM_RETRY:
+ case DID_REQUEUE:
+ case DID_SOFT_ERROR:
+ return -EBUSY;
+ default:
+ return -EIO;
+ }
}
-
- if (res)
- return -EIO;
return 0;
}
@@ -3062,9 +3082,17 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
sd_print_result(sdkp, res);
if (driver_byte(res) & DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
+ if ((scsi_sense_valid(&sshdr) &&
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a))
+ res = 0;
}
- return res;
+ /* SCSI error codes must not go to the generic layer */
+ if (res)
+ return -EIO;
+
+ return 0;
}
/*
@@ -3096,7 +3124,7 @@ exit:
scsi_disk_put(sdkp);
}
-static int sd_suspend(struct device *dev)
+static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
{
struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
int ret = 0;
@@ -3113,7 +3141,10 @@ static int sd_suspend(struct device *dev)
if (sdkp->device->manage_start_stop) {
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
+ /* an error is not worth aborting a system sleep */
ret = sd_start_stop_device(sdkp, 0);
+ if (ignore_stop_errors)
+ ret = 0;
}
done:
@@ -3121,6 +3152,16 @@ done:
return ret;
}
+static int sd_suspend_system(struct d
Re: Ejected Nook (usb mass storage) prevents suspend
On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote:
> More importantly, if we already know that the medium is not present or
> has been changed since it was last used, then there's no reason to call
> sd_sync_cache() at all.
Like this?
Regards
Oliver
>From 8c90d860652aa99e6e60d9b32bc3aa8d4db9efa5 Mon Sep 17 00:00:00 2001
From: Oliver Neukum
Date: Thu, 1 Aug 2013 10:08:20 +0200
Subject: [PATCH] sd: error handling during flushing caches
It makes no sense to flush the cache of a device without medium.
Errors during suspend must be handled according to their causes.
Errors due to missing media or unplugged devices must be ignored.
Errors due to devices being offlined must also be ignored.
The error returns must be modified so that the generic layer
understands them.
Signed-off-by: Oliver Neukum
---
drivers/scsi/scsi_pm.c | 3 ++-
drivers/scsi/sd.c | 69 ++
2 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index 4c5aabe..af4c050 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -54,7 +54,8 @@ scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
/*
* All the high-level SCSI drivers that implement runtime
* PM treat runtime suspend, system suspend, and system
- * hibernate identically.
+ * hibernate nearly identically. In all cases the requirements
+ * for runtime suspension are stricter.
*/
if (pm_runtime_suspended(dev))
return 0;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 86fcf2c..3c7f918 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -105,7 +105,8 @@ static void sd_unlock_native_capacity(struct gendisk *disk);
static int sd_probe(struct device *);
static int sd_remove(struct device *);
static void sd_shutdown(struct device *);
-static int sd_suspend(struct device *);
+static int sd_suspend_system(struct device *);
+static int sd_suspend_runtime(struct device *);
static int sd_resume(struct device *);
static void sd_rescan(struct device *);
static int sd_done(struct scsi_cmnd *);
@@ -483,11 +484,11 @@ static struct class sd_disk_class = {
};
static const struct dev_pm_ops sd_pm_ops = {
- .suspend = sd_suspend,
+ .suspend = sd_suspend_system,
.resume = sd_resume,
- .poweroff = sd_suspend,
+ .poweroff = sd_suspend_system,
.restore = sd_resume,
- .runtime_suspend = sd_suspend,
+ .runtime_suspend = sd_suspend_runtime,
.runtime_resume = sd_resume,
};
@@ -1437,6 +1438,8 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
if (!scsi_device_online(sdp))
return -ENODEV;
+ if (!sdkp->media_present)
+ return 0;
for (retries = 3; retries > 0; --retries) {
unsigned char cmd[10] = { 0 };
@@ -1455,12 +1458,31 @@ static int sd_sync_cache(struct scsi_disk *sdkp)
if (res) {
sd_print_result(sdkp, res);
- if (driver_byte(res) & DRIVER_SENSE)
+
+ if (driver_byte(res) & DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
+ /* we need to evaluate the error return */
+ if ((scsi_sense_valid(&sshdr) &&
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a))
+/* this is no error here */
+return 0;
+
+ switch (host_byte(res)) {
+ /* ignore errors due to racing a disconnection */
+ case DID_BAD_TARGET:
+ case DID_NO_CONNECT:
+ return 0;
+ /* signal the upper layer it might try again */
+ case DID_BUS_BUSY:
+ case DID_IMM_RETRY:
+ case DID_REQUEUE:
+ case DID_SOFT_ERROR:
+ return -EBUSY;
+ default:
+ return -EIO;
+ }
}
-
- if (res)
- return -EIO;
return 0;
}
@@ -3062,9 +3084,17 @@ static int sd_start_stop_device(struct scsi_disk *sdkp, int start)
sd_print_result(sdkp, res);
if (driver_byte(res) & DRIVER_SENSE)
sd_print_sense_hdr(sdkp, &sshdr);
+ if ((scsi_sense_valid(&sshdr) &&
+ /* 0x3a is medium not present */
+ sshdr.asc == 0x3a))
+ res = 0;
}
- return res;
+ /* SCSI error codes must not go to the generic layer */
+ if (res)
+ return -EIO;
+
+ return 0;
}
/*
@@ -3096,7 +3126,7 @@ exit:
scsi_disk_put(sdkp);
}
-static int sd_suspend(struct device *dev)
+static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
{
struct scsi_disk *sdkp = scsi_disk_get_from_dev(dev);
int ret = 0;
@@ -3107,13 +3137,20 @@ static int sd_suspend(struct device *dev)
if (sdkp->WCE) {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
ret = sd_sync_cache(sdkp);
- if (ret)
+ if (ret) {
+ /* ignore OFFLINE device */
+ if (ret == -ENODEV)
+ret = 0;
goto done;
+ }
}
if (sdkp->device->manage_start_stop) {
sd_printk(KERN_NOTICE, sdkp, "Stopping disk\n");
+ /* an error is not worth aborting a system sleep */
ret = sd_start_stop_device(sdkp, 0);
+ if (ignore_stop_errors)
+ ret = 0;
}
done:
@@ -3121,6 +3158,16 @@ done:
return ret
Re: Ejected Nook (usb mass storage) prevents suspend
On Thu, 2013-08-01 at 11:53 -0400, Alan Stern wrote: > On Thu, 1 Aug 2013, Oliver Neukum wrote: > > > On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote: > > > > > More importantly, if we already know that the medium is not present or > > > has been changed since it was last used, then there's no reason to call > > > sd_sync_cache() at all. > > > > Like this? > > Yes, I like this a lot better, except I would put the test for > !sdkp->media_present in sd_suspend_common() -- no need to print the But your observation that it makes no sense while no medium is present is valid whatever be the reason for wanting to flush. > "Synchronizing SCSI Cache" message if you're not going to go through > with it. > > What do the SCSI people think? I don't know. I would like Andy to test before I ask. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Hard disk S3 resume time optimization
On Thu, 2013-08-01 at 23:40 +, Brandt, Todd E wrote: > This patch is a potential way to reduce the S3 resume time for SATA drives. > Essentially this patch removes the hard disk resume time from the total > system resume time, with the disks still taking as long to come back online > but in the background. > > The major bottleneck is in the ata port resume which sends out a wakeup > command and then waits up to several seconds for the port to resume. This > patch changes the ata_port_resume_common function to be non blocking. The > other bottleneck is in the scsi disk resume code, which issues a a command to > startup the disk with blk_execute_rq, which then waits for the command to > finish (which also depends on the ata port being fully resumed). The patch > switches the sd_resume function to use blk_execute_rq_nowait instead (the > underlying code is identical to sd_resume, but is changed to non-blocking). But how do we know the command has succeeded? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Fri, 2013-08-02 at 09:54 -0400, Alan Stern wrote: > On Fri, 2 Aug 2013, Oliver Neukum wrote: > > > On Thu, 2013-08-01 at 11:53 -0400, Alan Stern wrote: > > > On Thu, 1 Aug 2013, Oliver Neukum wrote: > > > > > > > On Wed, 2013-07-31 at 11:13 -0400, Alan Stern wrote: > > > > > > > > > More importantly, if we already know that the medium is not present or > > > > > has been changed since it was last used, then there's no reason to > > > > > call > > > > > sd_sync_cache() at all. > > > > > > > > Like this? > > > > > > Yes, I like this a lot better, except I would put the test for > > > !sdkp->media_present in sd_suspend_common() -- no need to print the > > > > But your observation that it makes no sense while no medium is present > > is valid whatever be the reason for wanting to flush. > > So do the test in both places. Code duplication for what reasons? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Hard disk S3 resume time optimization
On Fri, 2013-08-02 at 16:41 +, Brandt, Todd E wrote: > Do you mean in terms of debug after a failure? I can resubmit the patch with > the scsi_sense_hdr buffer still in place. I just wanted to keep the first > draft simple to get the concept across. How are errors reported? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0
On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote: > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType *eeprom, > unsigned long io_port) >*/ > dprintkl(KERN_WARNING, > "EEProm checksum error: using default values and > options.\n"); > - eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM; > + eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff); Hi, if you are fixing these issues please use the proper macros for conversion of endianness. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] [SCSI] dc395x: use NULL instead of 0
On Wed, 2013-08-07 at 15:58 +0900, Jingoo Han wrote: > On Wednesday, August 07, 2013 3:50 PM, Oliver Neukum wrote: > > On Wed, 2013-08-07 at 12:55 +0900, Jingoo Han wrote: > > > > > @@ -4183,15 +4183,17 @@ static void check_eeprom(struct NvRamType > > > *eeprom, unsigned long io_port) > > >*/ > > > dprintkl(KERN_WARNING, > > > "EEProm checksum error: using default values and > > > options.\n"); > > > - eeprom->sub_vendor_id[0] = (u8)PCI_VENDOR_ID_TEKRAM; > > > + eeprom->sub_vendor_id[0] = (u8)(PCI_VENDOR_ID_TEKRAM & 0xff); > > > > Hi, > > > > if you are fixing these issues please use the proper macros for > > conversion of endianness. > > Sorry, I cannot understand exactly what you mean. :( > Would you please let me know which macros can be used? In this case constant_cpu_to_le16() would be the macro you want. Have a look at include/uapi/linux/byteorder/ Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Hard disk S3 resume time optimization
On Fri, 2013-08-09 at 01:09 +, Brandt, Todd E wrote:
> static struct ata_force_ent *ata_force_tbl;
> static int ata_force_tbl_size;
> +int ata_resume_status;
A single global variable for multiple ports?
> static char ata_force_param_buf[PAGE_SIZE] __initdata;
> /* param_buf is thrown away after initialization, disallow read */
> @@ -5415,6 +5416,22 @@ static int ata_port_resume(struct device *dev)
> return rc;
> }
>
> +static int ata_port_resume_async(struct device *dev)
> +{
> + struct ata_port *ap = to_ata_port(dev);
> +
> + ata_resume_status = 0;
> + ata_port_request_pm(ap, PMSG_RESUME, ATA_EH_RESET,
> + ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, &ata_resume_status);
If this ever runs in paralell it is extremely racy.
> + if (!ata_resume_status) {
> + pm_runtime_disable(dev);
> + pm_runtime_set_active(dev);
> + pm_runtime_enable(dev);
> + }
> +
> + return ata_resume_status;
> +}
> +
> + req = blk_get_request(sdkp->device->request_queue, 0, __GFP_WAIT);
What happens if commands go to the device before this has finished?
Especially should it fail.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] Hard disk S3 resume time optimization
On Fri, 2013-08-23 at 14:43 -0700, Brandt, Todd E wrote: > This is v3 of the non-blocking S3 resume patch. It's been broken into > two pieces, this part is for the scsi subsystem. I've addressed Alan > Stern's comments in particular by reformatting the call to conform > to the proper style guidelines. Hi, you are changing the generic resume for all sd disks. In particular, you are dropping the check of sdkp->device->manage_start_stop What happens if you use this patch on a real SCSI disk? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] uas: add dead request list
On Mon, 2013-09-02 at 13:25 +0200, Gerd Hoffmann wrote:
> +static void uas_zap_dead(struct uas_dev_info *devinfo)
> +{
> + struct uas_cmd_info *cmdinfo;
> + struct uas_cmd_info *temp;
> + struct list_head list;
> + unsigned long flags;
> +
> + spin_lock_irq(&uas_lists_lock);
> + list_replace_init(&uas_dead_list, &list);
> + spin_unlock_irq(&uas_lists_lock);
This looks like a window for a race.
> + spin_lock_irqsave(&devinfo->lock, flags);
> + list_for_each_entry_safe(cmdinfo, temp, &list, dead) {
> +
What happens if list entries are on the private list, when
the function is called for another device? It looks to me like
the di==devinfo test could put them back on the list although
they would need to be canceled.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
READ_CAPACITY_16 vs. READ_CAPACITY_10
Hi Hannes, you objected to this patch saying there's a possibilty that HS devices may also need this feature, which would require a quirk. Does this mean that the patch is acceptable only with an additional predefined quirk, or do you insist that all devices be handled with quirks? Regards Oliver +++ b/drivers/usb/storage/scsiglue.c @@ -211,8 +211,11 @@ static int slave_configure(struct scsi_device0*sdev) /* * Many devices do not respond properly to READ_CAPACITY_16. * Tell the SCSI layer to try READ_CAPACITY_10 first. +* However some USB 3.0 drive enclosures return capacity +* modulo 2TB */ - sdev->try_rc_10_first = 1; + if (us->pusb_dev->speed < USB_SPEED_SUPER) + sdev->try_rc_10_first = 1; /* assume SPC3 or latter devices support sense size > 18 */ if (sdev->scsi_level > SCSI_SPC_2) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Tue, 2013-09-10 at 13:25 -0400, Alan Stern wrote: > On Tue, 10 Sep 2013, Oliver Neukum wrote: > > > Hi Hannes, > > > > you objected to this patch saying there's a possibilty that > > HS devices may also need this feature, which would require > > a quirk. Does this mean that the patch is acceptable only > > with an additional predefined quirk, or do you insist that all > > devices be handled with quirks? > > Indeed, we already know of one or two high-speed devices that suffer > from this bug: > > http://marc.info/?l=linux-usb&m=133586313307042&w=2 > > This may influence your decision. I'm not certain whether it is > important enough to merit a new quirk flag, but people experiencing the > problem may have some strong opinions. What is the alternative? I think we can be sure that no drive enclosure will crash with READ_CAPACITY_16. I am not sure about card readers. Does anybody know what Windows does? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Wed, 2013-09-11 at 10:14 -0400, Alan Stern wrote: > There are three possibilities: nothing, your proposed patch, and a new Nothing is feasible only if Windows uses READ_CAPACITY_10. > quirk flag. The flag is safest, but also the hardest to maintain. Again the same answer. > > I think we can be sure that no drive enclosure will crash > > with READ_CAPACITY_16. > > I wouldn't count on it, but I don't know of any examples. > > > I am not sure about card readers. > > Or flash drives. > > > Does anybody know what Windows does? > > Not me. It probably varies with different versions of Windows. I'll try to get a Windows machine for a trace. Can you suggest a tracer for Win7? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Ejected Nook (usb mass storage) prevents suspend
On Fri, 2013-09-13 at 06:37 -0700, Andy Lutomirski wrote: > What's the status of this? Do I still need to test it? > > I won't have access to the Nook that triggered it the first time for a > couple weeks, but I could try to find another device like that. It works for me. I'll push upstream in a form that Alan likes. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: READ_CAPACITY_16 vs. READ_CAPACITY_10
On Thu, 2013-09-12 at 07:47 +0200, Hannes Reinecke wrote: > On 09/11/2013 04:14 PM, Alan Stern wrote: > > On Tue, 10 Sep 2013, Oliver Neukum wrote: > >> I think we can be sure that no drive enclosure will crash > >> with READ_CAPACITY_16. > > > > I wouldn't count on it, but I don't know of any examples. > > > Me neither. The whole issue just smells of some firmware coders > messing up their stuff. So I would think that other firmwares > might not have this problem. Certainly. But does it matter? We would be happy to always use READ_CAPACITY_16 first, if it weren't for other buggy firmware. > (But as HS enclosures aren't that common chances are we've hit > the same firmware by chance on every test machine we've had.) > > I think it would warrant a quirk approach. Yes, it might be slightly > more coding, but at the same time it's more selective on where it > should trigger. Yes and we would for sure miss many cases. > Also looking at scsiglue.c it would make things quite tricky if > you'd want to _disable_ this feature for HS devices; other firmware Just another quirk. > from other vendors might not exhibit this issue after all. > And scsiglue.c already has tons of quirks set, adding one more > doesn't do any harm. It doesn't do harm if the number of devices to be added is small. The enclosure works under Windows. So we know that it uses READ_CAPACITY_16 under some circumstances. Unfortunately I haven't dug up a Windows box as yet. However we can realistically hope that the firmwares become fixed only if Windows _fails_ to use READ_CAPACITY_16 under some circumstances. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/4] sd: change to auto suspend mode
On Sunday 06 January 2013 16:41:37 Aaron Lu wrote: > From: Lin Ming > > Uses block layer runtime pm helper functions in > scsi_runtime_suspend/resume. > Remove scsi_autopm_* from sd open/release path and check_events path. > And remove the quiesce call in runtime suspend path, as we know there is > no request to quiesce for the device. How does this handle ioctl() ? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 1/9] scsi: sr: support runtime pm
On Monday 21 January 2013 17:11:04 Aaron Lu wrote: > It is not easy for the OS to tell if the drive is being used or not > sometimes > > Alan has reminded me it is possible for an app to open the block device > file(/dev/sr0), issue a command(play audio), then close the device file. > From the OS' point of view, we think nobody is using it. But actually, > the drive is playing cd for the user, so we can't suspend the device. Are there drives that support ZPODD and have an audio output? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v13 1/9] scsi: sr: support runtime pm
On Tuesday 22 January 2013 10:25:31 Aaron Lu wrote: > On Mon, Jan 21, 2013 at 03:56:43PM +0100, Oliver Neukum wrote: > > On Monday 21 January 2013 17:11:04 Aaron Lu wrote: > > > It is not easy for the OS to tell if the drive is being used or not > > > sometimes > > > > > > Alan has reminded me it is possible for an app to open the block device > > > file(/dev/sr0), issue a command(play audio), then close the device file. > > > From the OS' point of view, we think nobody is using it. But actually, > > > the drive is playing cd for the user, so we can't suspend the device. > > > > Are there drives that support ZPODD and have an audio output? > > I'm afraid I don't know, since there are so many ODD makers. > But at least we can say, the SPEC doesn't forbid it. Well, then we have to handle it. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: 2TB USB hard drive for backing up
On Tuesday 22 January 2013 11:05:35 James Bottomley wrote: > > May 3 18:19:06 relampago3 kernel: [ 3948.472796] sd 7:0:0:0: [sdf] > > 1565565872 512-byte logical blocks: (801 GB/746 GiB) > > This looks like a wrap around of your actual size. This appears to > indicate the device isn't replying correctly to READ CAPACITY. The > conventional return from READ CAPACITY should be -1 which would trigger > us to retry with READ CAPACITY(16). What can we do? The answer the device is giving is quite ordinary. Do we need another quirk and an associated flag? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] [SCSI] dc395x: uninitialized variable in device_alloc()
On Monday 11 February 2013 22:03:18 Dan Carpenter wrote: > This bug was introduced back in bitkeeper days in 2003. We use > "dcb->dev_mode" before it has been initialized. > > Signed-off-by: Dan Carpenter Acked-by: Oliver Neukum -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: storage: add "no SYNCHRONIZE CACHE" quirk
On Thu, 2015-12-03 at 13:36 -0500, Alan Stern wrote: > This is an old problem, but it was never resolved and it still affects > people (Bugzilla #89511). In short, there are USB-(S)ATA bridges that > claim to be write-back but don't support the SYNCHRONIZE CACHE > command. > This causes errors when filesystems try to flush data out to the disk. OK, maybe this is a stupid idea, but could we test the command as we enumerate the slave and store the result? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] Re: [PATCH 04/14] uas: lock the abort handler
On Tue, 2016-01-26 at 10:28 -0500, Alan Stern wrote: > On Tue, 26 Jan 2016, Oliver Neukum wrote: > > That is problematic. The ABORT_TMF does need IO for which we need > > to wait. And if we just submit and pretend the abort worked, the tag > > will be reused. > > Perhaps we could drop the spinlock. I think it is time to talk to > > the SCSI people. > > The abort handler doesn't have to wait for the command to terminate. > It merely has to initiate an abort and return. The SCSI layer gets > notified via the usual mechanism when the command eventually finishes, > whether it was successful or not. This is stange. I don't see how we can guarantee a command completion if eh_abort_handler() has sent the TMF. It was my understanding that returning from eh_abort_handler() means that a tag is considered free again and will be reused. Could you clarify? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Resend PATCH 3/3] scsi:stex.c Add S3/S4 support
On Thu, 2016-02-04 at 19:22 +0800, Charles Chiou wrote:
> +static int stex_choice_sleep_mic(pm_message_t state)
> +{
> +switch (state.event) {
> +case PM_EVENT_SUSPEND:
> +return ST_S3;
> +case PM_EVENT_FREEZE:
Why do you react to PM_EVENT_FREEZE at all?
That is too early. You will get a HIBERNATE
event anyway. If the write out fails you are
in trouble if you already reacted to
PM_EVENT_FREEZE
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SCSI dynamic power management
Am Dienstag 20 November 2007 schrieb James Bottomley: > I don't understand why you want to do this. Power management is a > layered issue on SCSI, divided (as always) into host, device and > transport. The idle you're talking about is a pure device thing, so it > can be managed by the ULD (and currently is). When a unit is stopped, The lower layers don't know how to correctly suspend a device. sd_suspend() may know how to do it. It would also mean the LLD having to detect idleness. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SCSI dynamic power management
Am Dienstag 20 November 2007 schrieb James Bottomley: > > On Tue, 2007-11-20 at 16:07 +0100, Oliver Neukum wrote: > > Am Dienstag 20 November 2007 schrieb James Bottomley: > > > I don't understand why you want to do this. Power management is a > > > layered issue on SCSI, divided (as always) into host, device and > > > transport. The idle you're talking about is a pure device thing, so it > > > can be managed by the ULD (and currently is). When a unit is stopped, > > > > The lower layers don't know how to correctly suspend a device. sd_suspend() > > may know how to do it. It would also mean the LLD having to detect idleness. > > You really mean you want to involve the transport as well, right? So Yes, we cannot avoid that. Some device drop their caches unless they are flushed. > ipso facto this is more than simple idleness management. Thus, you > really need to look into the full solution (host, transport and ULD). Only detecting idleness without doing anything with that knowledge would be pointless :-) What is the right kind of sequence? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > When all the devices under a host are suspended, the LLD is informed > (via a new "autosuspend" method in the host template) so that it can That is most certainly a mistake. Is there a good reason to not modify to extend suspend() to take an extra argument for the reason it is called? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Montag, 7. Januar 2008 22:34:52 schrieb Alan Stern: > On Mon, 7 Jan 2008, Oliver Neukum wrote: > > > Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > > > When all the devices under a host are suspended, the LLD is informed > > > (via a new "autosuspend" method in the host template) so that it can > > > > That is most certainly a mistake. > > Why? You'll have to add this method whenever a new subsystem is affected by power management. Besides what are the semantics of this method? May suspend() be called after autosuspend() ? If not, will resume() be called or autoresume() ? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Dienstag, 8. Januar 2008 04:56:03 schrieb Alan Stern: > > You'll have to add this method whenever a new subsystem is affected > > by power management. > > Sorry, I don't understand your point. If you mean that we'll have to > add autosuspend and autoresume code to every driver that wants to > support autosuspend, I agree. There doesn't seem to be any way around > it. Note that the code added by the companion usb-storage patch is > pretty small. OK, you've convinced me. But I'd be happier if you put the notification into the generic device code. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > /** > + * autoresume - perform dynamic (runtime) host resume > + [EMAIL PROTECTED]: host to resume > + * > + * Resume (return to an operational power level) the specified host. > + * Return 0 if the resume was successful, otherwise a negative > + * error code. > + * > + * Locks: struct Scsi_Host::pm_mutex held throughout the call. > + * > + * Calling context: process > + * > + * Notes: If the host is not currently suspended, this method does > + * need to do anything. > + * > + * Optionally defined in: LLD > + **/ > + int autoresume(struct Scsi_Host *shp) > + This seems to be a bit misleading. It seems to me that you must cancel any outstanding request to autosuspend from other layers. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Dienstag, 8. Januar 2008 16:06:53 schrieb Alan Stern: > Eventually parts of the autosuspend mechanism will go there. But first > I thought we should have a proof-of-concept version working for at > least two different subsystems (such as SCSI and USB), so that we can > understand what should go into the generic layer and what should stay > out. Fine by me. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Dienstag, 8. Januar 2008 16:12:52 schrieb Alan Stern: > On Tue, 8 Jan 2008, Oliver Neukum wrote: > > > Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > > > /** > > > + * autoresume - perform dynamic (runtime) host resume > > > + [EMAIL PROTECTED]: host to resume > > > + * > > > + * Resume (return to an operational power level) the specified host. > > > + * Return 0 if the resume was successful, otherwise a negative > > > + * error code. > > > + * > > > + * Locks: struct Scsi_Host::pm_mutex held throughout the call. > > > + * > > > + * Calling context: process > > > + * > > > + * Notes: If the host is not currently suspended, this method does > > > + * need to do anything. > > > + * > > > + * Optionally defined in: LLD > > > + **/ > > > + int autoresume(struct Scsi_Host *shp) > > > + > > > > This seems to be a bit misleading. It seems to me that you must cancel > > any outstanding request to autosuspend from other layers. > > How about something more like this: > > * Resume (return to an operational power level) the specified host, > * and prevent autosuspends from other software layers until the > * template autosuspend method has been called again. > * Return 0 if the resume was successful, otherwise a negative > * error code. Much better. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > This patch applies to 2.6.24-rc6. Comments and suggestions are > welcome. What about the SG_IO ioctl() ? It seems to me that you must not autosuspend a device after that ioctl() has been used until the file handle is closed. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Dienstag, 8. Januar 2008 16:16:43 schrieb Alan Stern: > > What about the SG_IO ioctl() ? It seems to me that you must not autosuspend > > a device after that ioctl() has been used until the file handle is closed. > > That's an open problem. The patch does block autosuspends as long as > an sg file handle is open, but that won't help with ioctl requests > through the block device. > > Can you write an additional patch to fix this? I'll work on that. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Montag, 7. Januar 2008 20:42:23 schrieb Alan Stern: > When all the devices under a host are suspended, the LLD is informed > (via a new "autosuspend" method in the host template) so that it can > put the host adapter in a low-power state. Likewise, a new > "autoresume" method is called when the host adapter needs to return to > full power. An implementation of these methods for the usb-storage > driver will be posted in a followup patch. This has an interesting implication. As the storage driver can share a device with in principle any other usb driver, we must audit all usb drivers if we wish to adopt this patch. All a device's interfaces must be resumed when the storage interface is resumed. To resume a storage device no memory must be allocated because that could deadlock. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Mittwoch, 9. Januar 2008 18:22:51 schrieb Alan Stern: > On Wed, 9 Jan 2008, Oliver Neukum wrote: > > > This has an interesting implication. As the storage driver can share > > a device with in principle any other usb driver, we must audit all usb > > drivers if we wish to adopt this patch. > > All a device's interfaces must be resumed when the storage interface > > is resumed. To resume a storage device no memory must be allocated > > because that could deadlock. > > Maybe people shouldn't enable autosuspend for their swap device... Good advice, but not sufficient to avoid this problem. The vm may write out normal dirty cached pages to scsi devices, which affects storage. > What happens during normal system resume if a driver (not just USB!) > needs to allocate memory before the swap device has been resumed? I have no idea. I guess these code paths have a sync in the suspend path, so a lot of clean pages will be available. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
question on sd_open()
Hello, is there a way to distinguish calls to sd_open() from mount() from calls to open() ? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Implementation of SCSI dynamic power management
Am Mittwoch, 9. Januar 2008 21:36:20 schrieb Alan Stern: > On Wed, 9 Jan 2008, Oliver Neukum wrote: > > > Am Mittwoch, 9. Januar 2008 18:22:51 schrieb Alan Stern: > > > On Wed, 9 Jan 2008, Oliver Neukum wrote: > > > > > > > This has an interesting implication. As the storage driver can share > > > > a device with in principle any other usb driver, we must audit all usb > > > > drivers if we wish to adopt this patch. > > > > All a device's interfaces must be resumed when the storage interface > > > > is resumed. To resume a storage device no memory must be allocated > > > > because that could deadlock. > > > > > > Maybe people shouldn't enable autosuspend for their swap device... > > > > Good advice, but not sufficient to avoid this problem. The vm may write > > out normal dirty cached pages to scsi devices, which affects storage. > > For now, I think the best approach is "head-in-the-sand". There aren't > a lot of USB storage devices partnered with other functions at the > moment. Very well with exception of the hub driver. > But it might be a good idea for all USB drivers to use GFP_NOIO in > their resume pathways. Yes, we should make this a requirement for every driver henceforth changed to support autosuspend. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
memory allocation in sg_io()
Hi,
could you explain to me why this code can get away with allocating the
sense buffer on the stack?
static int sg_io(struct file *file, struct request_queue *q,
struct gendisk *bd_disk, struct sg_io_hdr *hdr)
{
unsigned long start_time;
int writing = 0, ret = 0, has_write_perm = 0;
struct request *rq;
char sense[SCSI_SENSE_BUFFERSIZE];
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: memory allocation in sg_io()
Am Donnerstag, 10. Januar 2008 14:05:25 schrieb Boaz Harrosh:
> On Thu, Jan 10 2008 at 14:33 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > could you explain to me why this code can get away with allocating the
> > sense buffer on the stack?
> >
> > static int sg_io(struct file *file, struct request_queue *q,
> > struct gendisk *bd_disk, struct sg_io_hdr *hdr)
> > {
> > unsigned long start_time;
> > int writing = 0, ret = 0, has_write_perm = 0;
> > struct request *rq;
> > char sense[SCSI_SENSE_BUFFERSIZE];
> >
> > Regards
> > Oliver
> > -
> where? what? do you mean in scsi_ioctl.c?
> why not it's a synchronous call?
> Do you mean 96 bytes is too big?
> Do you mean DMA alignment and cache coherency? I'm working
> on that for scsi devices.
Yes, you are doing DMA on the stack.
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] 2.6.24-git usb reset problems
Am Dienstag, 29. Januar 2008 15:11:08 schrieb Jens Axboe: > On Tue, Jan 29 2008, Boaz Harrosh wrote: > > On Tue, Jan 29 2008 at 15:54 +0200, Jens Axboe <[EMAIL PROTECTED]> wrote: > > > On Tue, Jan 29 2008, Boaz Harrosh wrote: > > >> Greg KH wrote: > > > No difference, still just a lot of resets. > > > > > Where you able to figure out which usb storage transport is used? > > > > in drivers/usb/storage/usb.c you have get_protocol() and get_transport() > > functions. I'm not sure if these get stored in sysfs perhaps. This will > > pinpoint better where to look. Let me research a bit. > > Did the quick'n easy and dumped it. Protocol is 'Transparent SCSI' and > transport is 'Bulk' You can recompile your kernel with CONFIG_USB_DEBUG and CONFIG_STORAGE_DEBUG That should tell the reason for the resets. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] 2.6.24-git usb reset problems
Am Dienstag, 29. Januar 2008 16:50:35 schrieb Boaz Harrosh: > On Tue, Jan 29 2008 at 16:31 +0200, Oliver Neukum <[EMAIL PROTECTED]> wrote: > I can not see what it is. Yes CONFIG_USB_STORAGE_DEBUG will help. > I have a device here that uses the same trasport/protocol I'll > try to reproduce the failure here. This is the most common combination of transport and protocol. If it were broken in a larger number of cases, we'd hear more reports. You'll probably need the debug output to figure out what's wrong. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] drivers/scsi/dc395x.c - msgin_qtag()
On Fri, 2013-11-15 at 17:53 -0200, Geyslan Gregório Bem wrote:
Hi,
> Hi guys,
>
> In the function msgin_qtag() [line 2632], this dereference was intentional?
>
> static struct ScsiReqBlk *msgin_qtag(struct AdapterCtlBlk *acb,
> struct DeviceCtlBlk *dcb, u8 tag)
> {
> struct ScsiReqBlk *srb = NULL;
> struct ScsiReqBlk *i;
> dprintkdbg(DBG_0, "msgin_qtag: (0x%p) tag=%i srb=%p\n",
>srb->cmd, tag, srb);
> ...
>
> There is a srb (NULL) dereference in the dprintkdbg() parameteres.
That is a bad bug.
> If not, what approach do you suggest me for a patch?
Merge it with
dprintkdbg(DBG_0, "msgin_qtag: (0x%p) <%02i-%i>\n",
srb->cmd, srb->dcb->target_id, srb->dcb->target_lun);
later in the function.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Disk spin-up optimization during system resume
On Thu, 2014-01-16 at 11:59 -0500, Alan Stern wrote: > Since the START-STOP and TEST UNIT READY (or REQUEST SENSE or > whatever) > commands are likely to take a long time, they should all be carried > out > asynchronously with respect to the resume procedure. I don't know > what > the best way is to implement this. But it is important to guarantee > that in the RPM_ACTIVE case, the START-STOP command gets sent to the > disk before any other commands. (This isn't an issue in the > RPM_SUSPENDED case, as the block layer will prevent requests being > sent out unless they have the REQ_PM flag set.) > > Does this plan sound reasonable to everyone? Are there important > aspects I haven't considered (such as interactions between the SCSI > and > ATA layers)? The START-STOP may result in an error. What do you do in that case? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup
Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh: > > - use scsi_cmnd data accessors > - Clean the !use_sg code paths This doesn't compile, at least on x86_64, so NAK. Regards Oliver CC [M] drivers/usb/image/microtek.o drivers/usb/image/microtek.c: In function ‘mts_data_done’: drivers/usb/image/microtek.c:450: warning: implicit declaration of function ‘scsi_set_resid’ drivers/usb/image/microtek.c: In function ‘mts_command_done’: drivers/usb/image/microtek.c:494: warning: implicit declaration of function ‘scsi_sg_count’ drivers/usb/image/microtek.c: In function ‘mts_do_sg’: drivers/usb/image/microtek.c:517: warning: implicit declaration of function ‘scsi_sglist’ drivers/usb/image/microtek.c:517: warning: assignment makes pointer from integer without a cast drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’: drivers/usb/image/microtek.c:553: warning: implicit declaration of function ‘scsi_bufflen’ drivers/usb/image/microtek.c:558: warning: assignment makes pointer from integer without a cast MODPOST vmlinux Kernel: arch/x86_64/boot/bzImage is ready (#4) Building modules, stage 2. MODPOST 1705 modules ERROR: "scsi_set_resid" [drivers/usb/image/microtek.ko] undefined! ERROR: "scsi_sg_count" [drivers/usb/image/microtek.ko] undefined! ERROR: "scsi_sglist" [drivers/usb/image/microtek.ko] undefined! ERROR: "scsi_bufflen" [drivers/usb/image/microtek.ko] undefined! make[1]: *** [__modpost] Fehler 1 make: *** [modules] Fehler 2 - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] microtek.c - use data accessors and !use_sg cleanup
Am Donnerstag, 12. Juli 2007 schrieb James Bottomley: > On Thu, 2007-07-12 at 15:29 +0200, Oliver Neukum wrote: > > Am Donnerstag, 12. Juli 2007 schrieb Boaz Harrosh: > > > > > > - use scsi_cmnd data accessors > > > - Clean the !use_sg code paths > > > > This doesn't compile, at least on x86_64, so NAK. > > > > Regards > > Oliver > > > > CC [M] drivers/usb/image/microtek.o > > drivers/usb/image/microtek.c: In function ‘mts_data_done’: > > drivers/usb/image/microtek.c:450: warning: implicit declaration of function > > ‘scsi_set_resid’ > > drivers/usb/image/microtek.c: In function ‘mts_command_done’: > > drivers/usb/image/microtek.c:494: warning: implicit declaration of function > > ‘scsi_sg_count’ > > drivers/usb/image/microtek.c: In function ‘mts_do_sg’: > > drivers/usb/image/microtek.c:517: warning: implicit declaration of function > > ‘scsi_sglist’ > > drivers/usb/image/microtek.c:517: warning: assignment makes pointer from > > integer without a cast > > drivers/usb/image/microtek.c: In function ‘mts_build_transfer_context’: > > drivers/usb/image/microtek.c:553: warning: implicit declaration of function > > ‘scsi_bufflen’ > > drivers/usb/image/microtek.c:558: warning: assignment makes pointer from > > integer without a cast > > These would appear to be because you're not building against scsi-misc > rather than because of a failure in the patch. Thanks. Which tree should I test against? Where do I get it? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
question on flushing buffers and spinning down disk
Hi, which function should a lldd call to make the scsi layer flush a device's buffers and spin it down? Which kind of locking is required? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on flushing buffers and spinning down disk
Am Dienstag 18 September 2007 schrieb James Bottomley: > On Tue, 2007-09-18 at 10:32 +0200, Oliver Neukum wrote: > > which function should a lldd call to make the scsi layer flush > > a device's buffers and spin it down? Which kind of locking is > > required? > > Depends on the context. Is this for suspend? If so it's done > automatically by the sd driver, but the device has to be marked for it > in the manage_start_stop attributes. It is for runtime power management. We've gotten a bug report about a drive enclosure that doesn't properly park heads if the usb device is simply suspended. Apparently it simply cuts power so the cache can be lost, too. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on flushing buffers and spinning down disk
Am Dienstag 18 September 2007 schrieb James Bottomley:
> On Tue, 2007-09-18 at 16:15 +0200, Oliver Neukum wrote:
> > Am Dienstag 18 September 2007 schrieb James Bottomley:
> > > On Tue, 2007-09-18 at 10:32 +0200, Oliver Neukum wrote:
> > > > which function should a lldd call to make the scsi layer flush
> > > > a device's buffers and spin it down? Which kind of locking is
> > > > required?
> > >
> > > Depends on the context. Is this for suspend? If so it's done
> > > automatically by the sd driver, but the device has to be marked for it
> > > in the manage_start_stop attributes.
> >
> > It is for runtime power management. We've gotten a bug report about
> > a drive enclosure that doesn't properly park heads if the usb device is
> > simply suspended. Apparently it simply cuts power so the cache can
> > be lost, too.
>
> But even for runtime, if you want to suspend the device, shouldn't you
> be calling the suspend methods in the device tree?
Very good question. It seems to that yes indeed, we should. But we don't
in case of autosuspend. We simply suspend the interfaces:
static int usb_suspend_both(struct usb_device *udev, pm_message_t msg)
[..]
/* Suspend all the interfaces and then udev itself */
if (udev->actconfig) {
for (; i < udev->actconfig->desc.bNumInterfaces; i++) {
intf = udev->actconfig->interface[i];
status = usb_suspend_interface(intf, msg);
if (status != 0)
break;
}
}
without involving the driver core.
Is this a design bug?
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on flushing buffers and spinning down disk
Am Montag 24 September 2007 schrieb Alan Stern: > On Mon, 24 Sep 2007, Oliver Neukum wrote: > > > Am Dienstag 18 September 2007 schrieb James Bottomley: > > > On Tue, 2007-09-18 at 16:15 +0200, Oliver Neukum wrote: > > > > > It is for runtime power management. We've gotten a bug report about > > > > a drive enclosure that doesn't properly park heads if the usb device is > > > > simply suspended. Apparently it simply cuts power so the cache can > > > > be lost, too. > > > > > > But even for runtime, if you want to suspend the device, shouldn't you > > > be calling the suspend methods in the device tree? > > > > Very good question. It seems to that yes indeed, we should. But we don't > > in case of autosuspend. We simply suspend the interfaces: > > It's a loaded question. The fact is, the existing suspend methods in > the device tree are intended for system-wide suspend, not for runtime Unfortunately true. > suspend. The PM and driver cores don't include _any_ provision for > runtime suspend; it has to be managed separately by each subsystem. There we have a problem. We'd have to solve it for each subsystem transition. Storage has this type of children, other devices will have to care for other types of child devices. If we have a device tree, we should use it. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on flushing buffers and spinning down disk
Am Montag 24 September 2007 schrieb Alan Stern: > On Mon, 24 Sep 2007, Oliver Neukum wrote: > > > > suspend. The PM and driver cores don't include _any_ provision for > > > runtime suspend; it has to be managed separately by each subsystem. > > > > There we have a problem. We'd have to solve it for each subsystem > > transition. > > Storage has this type of children, other devices will have to care for other > > types of child devices. If we have a device tree, we should use it. > > It sounds like you want to add the equivalent of > usb_autopm_get/set_device() and usb_autopm_get/set_interface() into the That would be an ideal solution, but it is not necessary to solve this problem. > driver model. But it would not be appropriate, since other subsystems > don't have the same kinds of runtime power levels as USB does. But we don't bother when suspending the whole system. So why not simply walk the subtrees under a USB device? Let the subsystem choose what depth of sleep to use. > There may be no way of getting around the need for specialized > communication methods between subsystems for runtime PM. That's a lot of code duplication and should be a solution of last resort. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Montag 24 September 2007 schrieb Alan Stern: > On Mon, 24 Sep 2007, Oliver Neukum wrote: > > But we don't bother when suspending the whole system. So why not simply > > walk the subtrees under a USB device? Let the subsystem choose what > > depth of sleep to use. > > Because by doing so you are effectively telling the drivers in the > subtree that the entire system is going to be suspended, which isn't Unfortunately, that is correct. > correct. It may also turn out that the user wants to keep the > subsystem active, even though the USB driver isn't aware of it. Last > but not least, the subsystem will need to have some form of autoresume. Not necessarily. > The way it should work is for the lower subsystem to tell the USB > driver when suspending is okay, not the other way around. When the Absolutely. > subsystem implements its own form of runtime PM support, then you'll be > able to use it properly. But until then, there isn't anything much you > can do. Well, we can't simply cut power. Buffers must be flushed and the disk spun down. The suspend() method of the storage driver will have to become complex. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Montag 24 September 2007 schrieb Alan Stern: > On Mon, 24 Sep 2007, Oliver Neukum wrote: > > > > subsystem implements its own form of runtime PM support, then you'll be > > > able to use it properly. But until then, there isn't anything much you > > > can do. > > > > Well, we can't simply cut power. Buffers must be flushed and the disk > > spun down. The suspend() method of the storage driver will have to become > > complex. > > That's right, if the device is a real disk. If it's a flash memory > device then of course these issues don't arise. Unfortunately the > driver isn't able to tell one from the other; the user will have to > do that for us. So those devices which draw most power we don't suspend :-( Furthermore for many devices it will take real user intervention to determine what is in the enclosure. This isn't very good. I think we can do better. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
question on SDEV_BLOCK vs. SDEV_QUIESCE
Hi, what is the difference between these states? What function should I call to block a device in order to securely flush the caches? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Montag 24 September 2007 schrieb Alan Stern: > On Mon, 24 Sep 2007, Oliver Neukum wrote: > > > > subsystem implements its own form of runtime PM support, then you'll be > > > able to use it properly. But until then, there isn't anything much you > > > can do. > > > > Well, we can't simply cut power. Buffers must be flushed and the disk > > spun down. The suspend() method of the storage driver will have to become > > complex. > > That's right, if the device is a real disk. If it's a flash memory > device then of course these issues don't arise. Unfortunately the With respect to buffers how sure are you about that? > driver isn't able to tell one from the other; the user will have to > do that for us. Code for what we need is in sd_suspend(). The only trouble is locking. We need to make sure no commands that dirty buffers are issued after flushing the buffers. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: question on SDEV_BLOCK vs. SDEV_QUIESCE
Am Dienstag 25 September 2007 schrieb Hannes Reinecke: > Oliver Neukum wrote: > > Hi, > > > > what is the difference between these states? What function should > > I call to block a device in order to securely flush the caches? > > > SDEV_BLOCK blocks _any_ commands from the midlayer; eg. if the LLDD > is doing some internal recovery. > SDEV_QUIESCE only blocks 'normal' data commands; others (like control > commands) can still get through. > > So for flushing you should use SDEV_QUIESCE. Thank you, that's very helpful. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Dienstag 25 September 2007 schrieb Alan Stern: > > Furthermore for many devices it will take real user intervention to > > determine what is in the enclosure. This isn't very good. I think > > we can do better. > > If by "we" you mean the Linux kernel in general, then I agree. If by > "we" you mean the USB developers, then I disagree. Help is needed from > the SCSI people. I am working on it. It seems to me that all necessary functions exist. Regards Oliver PS: Whoever designed klists is suffering from a case of overengineeringosis. - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
reversing quiescing from user space
Hi, I am getting an oops with a scsi disk put into quiesced state. Is there an easy way to reverse this from user space so I can cleanly reboot? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Dienstag 25 September 2007 schrieb Alan Stern:
> Getting all this to work will be tricky, because sd is a block device
> driven by requests issued through the block layer in atomic context,
> whereas suspend and resume require a process context. Probably the
> SCSI core would have to get involved in managing the synchronization.
Well,
here's a patch that seems to do what needs to be done to make it work.
Regards
Oliver
--- a/drivers/usb/storage/usb.c 2007-09-27 13:33:31.0 +0200
+++ b/drivers/usb/storage/usb.c 2007-09-27 13:30:21.0 +0200
@@ -182,33 +182,100 @@ static struct us_unusual_dev us_unusual_
static int storage_suspend(struct usb_interface *iface, pm_message_t message)
{
+ struct device *child;
struct us_data *us = usb_get_intfdata(iface);
+ struct Scsi_Host *host = us_to_host(us);
+ struct scsi_device *sdev, *sdev2 = NULL;
+ int err = 0;
+
+ /* In case of autosuspend we need to do extra work to flush
+* buffers and spin down disks. */
+ if (us->pusb_dev->auto_pm) {
+ US_DEBUGP("Autosuspend code path.\n");
+ /* block all devices against block and user space io */
+ shost_for_each_device(sdev, host) {
+ US_DEBUGP("Quiescing device.\n");
+ err = scsi_device_quiesce(sdev);
+ US_DEBUGP("Quiesced device with result %d.\n", err);
+ if (err < 0) {
+ sdev2 = sdev;
+ err = -EIO;
+ scsi_device_put(sdev);
+ goto err_unblock;
+ }
+ child = &sdev->sdev_gendev;
+ if (!child->driver || !child->driver->suspend)
+ continue;
+ US_DEBUGP("About to suspend disk.\n");
+ /* flush the buffers and spin down */
+ err = (child->driver->suspend)(child, message);
+ if (err < 0) {
+ US_DEBUGP("Could not suspend disk.\n");
+ sdev2 = sdev;
+ shost_for_each_device(sdev, host) {
+ if (sdev == sdev2) {
+ scsi_device_put(sdev);
+ break;
+ }
+ child = &sdev->sdev_gendev;
+ if (!child->driver ||
!child->driver->resume)
+ continue;
+ (child->driver->resume)(child);
+ }
+ err = -EIO;
+ sdev2 = NULL;
+ scsi_device_put(sdev);
+ goto err_unblock;
+ }
+ }
+ }
+
+ US_DEBUGP("%s\n", __FUNCTION__);
/* Wait until no command is running */
mutex_lock(&us->dev_mutex);
- US_DEBUGP("%s\n", __FUNCTION__);
if (us->suspend_resume_hook)
(us->suspend_resume_hook)(us, US_SUSPEND);
- /* When runtime PM is working, we'll set a flag to indicate
-* whether we should autoresume when a SCSI request arrives. */
-
mutex_unlock(&us->dev_mutex);
- return 0;
+
+ /* In case of autosuspend device must be unblocked again */
+ if (us->pusb_dev->auto_pm) {
+err_unblock:
+ shost_for_each_device(sdev, host) {
+ if (sdev == sdev2) {
+ scsi_device_put(sdev);
+ break;
+ }
+ scsi_device_resume(sdev);
+ }
+ }
+ return err;
}
static int storage_resume(struct usb_interface *iface)
{
struct us_data *us = usb_get_intfdata(iface);
+ struct Scsi_Host *host;
+ struct scsi_device *sdev;
+ struct device *child;
- mutex_lock(&us->dev_mutex);
+ if (us->pusb_dev->auto_pm) {
+ host = us_to_host(us);
+ shost_for_each_device(sdev, host) {
+ child = &sdev->sdev_gendev;
+ if (!child->driver || !child->driver->resume)
+ continue;
+ (child->driver->resume)(child);
+ }
+ }
US_DEBUGP("%s\n", __FUNCTION__);
+
if (us->suspend_resume_hook)
(us->suspend_resume_hook)(us, US_RESUME);
- mutex_unlock(&us->dev_mutex);
return 0;
}
@@ -306,6 +373,7 @@ static int usb_stor_control_thread(void
{
struct us_data *us = (struct us_data *)__us;
struct Scsi_Host *host =
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Donnerstag 27 September 2007 schrieb Alan Stern: > On Thu, 27 Sep 2007, Oliver Neukum wrote: > > > Am Dienstag 25 September 2007 schrieb Alan Stern: > > > Getting all this to work will be tricky, because sd is a block device > > > driven by requests issued through the block layer in atomic context, > > > whereas suspend and resume require a process context. Probably the > > > SCSI core would have to get involved in managing the synchronization. > > > > > > Well, > > > > here's a patch that seems to do what needs to be done to make it work. > > Oliver, I'm surprised at you! This patch is a big hack, not to mention > a layering violation and an inversion of the proper calling sequence. Well, 1. autosuspend should not be specific to USB 2. I like to view the generic SCSI code as a library 3. The low level drivers will have to decide whether to suspend devices, as the generic code doesn't know whether the bus is shared. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Donnerstag 27 September 2007 schrieb Alan Stern: > On Thu, 27 Sep 2007, Oliver Neukum wrote: > > > > Oliver, I'm surprised at you! This patch is a big hack, not to mention > > > a layering violation and an inversion of the proper calling sequence. > > > > Well, > > > > 1. autosuspend should not be specific to USB > > Correct. Which means support for it should be added to the SCSI > drivers. SCSI autosuspend shouldn't be kluged into usb-storage in a > way that basically ignores what the SCSI layer is doing. It isn't. The decision to autosuspend is based on activity on the bus. Now you might argue that the decision to do an autosuspend should always propagate upwards from the lowest level of the device tree. Then I ask why? > > 2. I like to view the generic SCSI code as a library > > That viewpoint might work out okay in some cases, but I don't think > it's tenable when dealing with high-level drivers like sd and sr. sg worries me. > > 3. The low level drivers will have to decide whether to suspend devices, > > as the generic code doesn't know whether the bus is shared. > > I'm having trouble understanding this; it's a bit ambiguous. Does > "devices" mean "SCSI devices", "host adapter devices", or both? Does SCSI devices > "generic code" refer to the SCSI core, the high-level SCSI drivers, or > something else? When you say the bus is "shared", do you mean there > are multiple SCSI devices attached to it or do you mean something else? No multiple hosts. Think Firewire or FibreChannel. We certainly cannot spin down a Firewire disk some other host is using just because we are done with it. > (I assume you mean something else, as the SCSI core certainly does know > when multiple SCSI devices are attached to a SCSI bus.) > > In any case, that's not how I would put it. The SCSI core and > high-level drivers will have to decide when they are not using SCSI > devices and the devices can be suspended. When all the devices the Err, why? After all, there's a central instance in userspace that decides that the whole system isn't used right now. I see no reason to not use this model for parts of the device tree. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Freitag 28 September 2007 schrieb Greg KH: > On Tue, Sep 25, 2007 at 05:11:00PM +0200, Oliver Neukum wrote: > > > > PS: Whoever designed klists is suffering from a case of overengineeringosis. > > No objection from me there. If you can think of a way to do list > traversal without taking locks, please, feel free to redo the whole > klist stuff. No, I just was happy to be able to avoid using them. But it is not just overengineered, it is also underengineered: int bus_for_each_dev(struct bus_type * bus, struct device * start, void * data, int (*fn)(struct device *, void *)) An interface should be designed to note where it fails, thus: int bus_for_each_dev(struct bus_type * bus, struct device ** current, void * data, int (*fn)(struct device *, void *)) Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Donnerstag 27 September 2007 schrieb Alan Stern: > Have you thought about how autoresume would fit into this picture? I > don't think you can rely on usb-storage telling the SCSI core to resume > devices when it gets handed a command, because the commands to spin-up > the drives would have to be transmitted first. (The sample patch you Actually, it is not a big question. By default sd_resume() is a nop. Usually the first SCSI command would return NOT_READY and SCSI core do a START_STOP_UNIT only then in response. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Donnerstag 27 September 2007 schrieb Alan Stern: > On Thu, 27 Sep 2007, Oliver Neukum wrote: > > > > > 1. autosuspend should not be specific to USB > > > > > > Correct. Which means support for it should be added to the SCSI > > > drivers. SCSI autosuspend shouldn't be kluged into usb-storage in a > > > way that basically ignores what the SCSI layer is doing. > > > > It isn't. The decision to autosuspend is based on activity on the bus. > > Now you might argue that the decision to do an autosuspend should > > always propagate upwards from the lowest level of the device tree. > > Then I ask why? > > Because that's how I have always thought of it... :-) > > You're right; there's no showstopping reason particular subsystems like > SCSI can't use a different approach. However bus activity alone isn't > a sufficient criterion. For example, suppose somebody sends a PLAY > AUDIO command to a SCSI cdrom drive. There won't be any bus activity > while the music is playing, but the device will be active and so it > shouldn't be suspended. Thus the sr driver would want to fail an > autosuspend call. sr isn't a device driver. Furthermore it is generic. It can't know whether any power saving measure would prevent some activity from being carried out. It seems to me that we need to be conservative. Any device that is opened by user space must not be autosuspended. But how to fit this into SCSI? > Anyway, if we do end up going this route then there should be a routine > added to the SCSI core which would call the suspend methods for all the > attached devices. This doesn't belong in usb-storage (and even worse, > it shouldn't be duplicated in all the other low-level drivers); rather, > usb-storage should simply be able to call the new routine. We are moving in circles. In an earlier thread you argued that a suspend call should not walk the device tree in case of runtime power management. > Have you thought about how autoresume would fit into this picture? I > don't think you can rely on usb-storage telling the SCSI core to resume > devices when it gets handed a command, because the commands to spin-up > the drives would have to be transmitted first. (The sample patch you > posted will deadlock: The other command can't begin until the spin-up > command completes, and the spin-up command won't get sent to > usb-storage until the original command completes.) In other words, > autoresume must be handled at the level of the SCSI core. That's a difficult one. I need to ponder this a bit. [..] > There's also a philosophical objection. Who is in a better position to > judge when a device like a SCSI drive should be autosuspended: its own > driver (sd) or someone else (usb-storage)? SCSI doesn't know about power management. It cannot, it is not a notion native to SCSI. It just knows how to safely put drives into a state other than fully active (flushing buffers/spin down) You are expecting too much of SCSI core. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Donnerstag 27 September 2007 schrieb Alan Stern: > There's also a philosophical objection. Who is in a better position to > judge when a device like a SCSI drive should be autosuspended: its own > driver (sd) or someone else (usb-storage)? Then a philosophical answer. The highest entity which understands what it is doing when using power management. Highest here to be understood not as a position in the device tree, but in the flow of information. That is in our case usb-storage. Sr or sd can't do it because they don't and can't understand power management. Now they might be asked to provide some helpers. An open count and notifications about the state of the queue would be obvious. Other suggestions? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Freitag 28 September 2007 schrieb Alan Stern: > On Fri, 28 Sep 2007, Oliver Neukum wrote: > > > Am Donnerstag 27 September 2007 schrieb Alan Stern: > > > There's also a philosophical objection. Who is in a better position to > > > judge when a device like a SCSI drive should be autosuspended: its own > > > driver (sd) or someone else (usb-storage)? > > > > Then a philosophical answer. The highest entity which understands what > > it is doing when using power management. Highest here to be understood > > not as a position in the device tree, but in the flow of information. > > In this sense, sd is higher than usb-storage, right? Because I/O > requests pass through sd first, then usb-storage, on their way to the > device. Yes, but sr doesn't know hardware. > My point is that when dealing with SCSI disks, sd understands the > implications of Power Management better than usb-storage does. > Similarly, when dealing with SCSI cd drives, sr understands the > implications of Power Management better than usb-storage. No. The capabilities of the hardware are determined by USB. USB imposes two strict rules. 1. The host cannot talk to a suspended device (keeping it suspended) 2. Power consumption is very limited It makes no sense to talk about power management of devices as such. You need to talk about devices on the bus type they are connected to. > Hence by your own argument, the SCSI high-level drivers should be > responsible for autosuspending their respective devices. usb-storage, > on the other hand, should be responsible only for suspending the > transport, since that's what it does understand. The weakest link of the chain determines the whole. > > That is in our case usb-storage. Sr or sd can't do it because they don't > > and can't understand power management. > > That's simply not true. If sd didn't understand Power Management then > sd_suspend() and sd_resume() would be empty. It understands on and off. The methods simply do what is to be done to safely switch off a disk. > > Now they might be asked to provide some helpers. An open count and > > notifications about the state of the queue would be obvious. Other > > suggestions? > > I still think you're trying to go about it all backwards. I am using the point where the logical flow of information and the device tree diverge. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Freitag 28 September 2007 schrieb Alan Stern:
> On Fri, 28 Sep 2007, Oliver Neukum wrote:
>
> > Am Donnerstag 27 September 2007 schrieb Alan Stern:
> > > Have you thought about how autoresume would fit into this picture? I
> > > don't think you can rely on usb-storage telling the SCSI core to resume
> > > devices when it gets handed a command, because the commands to spin-up
> > > the drives would have to be transmitted first. (The sample patch you
>
> I had another look at your patch. It calls scsi_device_quiesce(),
> which causes all normal requests to be deferred. Hence under normal
> operation the device would never autoresume -- user requests would be
> deferred and never sent to usb-storage.
Unless I am very mistaken, further down in storage_suspend, I call
+ /* In case of autosuspend device must be unblocked again */
+ if (us->pusb_dev->auto_pm) {
+err_unblock:
+ shost_for_each_device(sdev, host) {
+ if (sdev == sdev2) {
+ scsi_device_put(sdev);
+ break;
+ }
+ scsi_device_resume(sdev);
which again allows normal io, so the autoresume is triggered.
It may deadlock, you are right about that, but it is definitely triggered.
I verified that experimentally.
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Samstag 29 September 2007 schrieb Alan Stern:
> On Fri, 28 Sep 2007, Oliver Neukum wrote:
>
> > Unless I am very mistaken, further down in storage_suspend, I call
> >
> > + /* In case of autosuspend device must be unblocked again */
> > + if (us->pusb_dev->auto_pm) {
> > +err_unblock:
> > + shost_for_each_device(sdev, host) {
> > + if (sdev == sdev2) {
> > + scsi_device_put(sdev);
> > + break;
> > + }
> > + scsi_device_resume(sdev);
> >
> > which again allows normal io, so the autoresume is triggered.
> > It may deadlock, you are right about that, but it is definitely triggered.
> > I verified that experimentally.
>
> Yes, okay, sorry, I didn't read far enough into the patch.
>
> Still, doesn't it seem peculiar that to implement autosuspend you have
> to quiesce the device and then reactivate it? In fact, it's not clear
> why you want to call scsi_device_quiesce() in the first place. The
> fact that scsi_bus_suspend() does it isn't a good reason.
I can't have the buffers be dirtied after flushing them.
Regards
Oliver
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Samstag 29 September 2007 schrieb Alan Stern: > On Fri, 28 Sep 2007, Oliver Neukum wrote: > > > Am Freitag 28 September 2007 schrieb Alan Stern: > > > On Fri, 28 Sep 2007, Oliver Neukum wrote: > I don't know what that's supposed to mean. And whatever that means, it > must be equally true that usb-storage doesn't know CD drives. Or disk > drives, or flash memories, or SCSI tape drives, or ... On the whole, > I'd say that usb-storage's lack of knowledge about SCSI devices greatly > outweighs sr's lack of knowledge about hardware. Neither driver knows enough to completely do power management. > > > My point is that when dealing with SCSI disks, sd understands the > > > implications of Power Management better than usb-storage does. > > > Similarly, when dealing with SCSI cd drives, sr understands the > > > implications of Power Management better than usb-storage. > > > > No. The capabilities of the hardware are determined by USB. > > USB imposes two strict rules. > > > > 1. The host cannot talk to a suspended device (keeping it suspended) > > 2. Power consumption is very limited > > > > It makes no sense to talk about power management of devices as > > such. You need to talk about devices on the bus type they are connected > > to. > > You forget that the USB bus ends at the USB interface chip on the > device. From there on it's a different sort of bus. It might be ATA, > as in commonly-available USB-ATA converter boxes. It might be a > genuine SCSI bus, as in some USB-SCSI bridges. It might be an internal > flash-memory "bus". It might be a full-blown Linux gadget. And so on. > > The suspend done by usb-storage suspends only the USB part of the link. > It might or might not affect anything beyond the device's USB chip, > depending on how the device was designed. Yes, but do we care about we don't see anyway? > Your (1) is correct, since all communication must take place over the > USB link. Your (2) is wrong -- the _link_'s power consumption is > indeed limited, but the _device_ could be self-powered and so its power > consumption need not be affected by the link state. For instance in > the case of a USB-ATA drive enclosure, power consumption is affected > more by whether or not the disk is spinning than anything else. OK, yes, it is true only for bus powered devices. Do we really want to differentiate that far? I suggest we assume the worst case. [..] > Suppose we have devices A1 and A2, where A1 is the parent and A2 is the > child. In order to pass between the CPU and A2, data must flow through > A1. Let D1 and D2 be the drivers for A1 and A2. > > It may be that for D2 to communicate with A2, it has to ask D1 to send > or receive the data for it. Or it may be that once D1 has configured > A1, data can flow automatically through A1 with no need for further > involvement on D1's part. Would you say that in either case the flow > of information has diverged from the device tree? In the latter case it diverges. > Suppose D1 decides that it wants to suspend A1, thus breaking the data > link to A2. Shouldn't it get D2's permission first? Better yet, > shouldn't it wait until D2 tells it: "Okay, A2 is safely suspended and > I'm not going to talk to it for a while, you can suspend the link"? Ok, I should be much more explicit and try to to explain very clearly. In short: D1 should definitely get D2's permission. But waiting is not enough. Suppose we have all child devices in a suspended state. Then we might ask whether this is enough to be sure that you can the suspend the parent. Unfortunately it is not, as the bug report about the drive enclosure cutting power when suspended shows. Suspension in a higher layer can have effects that are different to suspension of all devices on a lower level. Therefore the higher level must ask the lower level to prepare itself. Ideally it would ask the lower level for permission to do an autosuspend. I'd like to change the API so that you can do that. But I don't think that the lower levels have to implement autosuspend on their own to have levels above them support autosuspend. Can you summarize your requirements for supporting autosuspend in the higher levels? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [linux-usb-devel] question on flushing buffers and spinning down disk
Am Samstag 29 September 2007 schrieb Alan Stern: > I disagree. That bug report shows that problems arise when we try to > suspend a parent without making sure the children are suspended first. > If the sd suspend method had already run then it would have been okay > for the enclosure to cut power. That is true. The question is who is to call the suspend method. > > Suspension in a higher layer can have effects that are different to > > suspension > > of all devices on a lower level. Therefore the higher level must ask the > > lower > > level to prepare itself. > > When the lower level is suspended then it is supposed to be prepared > for the higher layer to suspend. No additional preparation should be > needed. Yes. If it returns from suspend without error a driver must keep that guarantee. > (That's true for USB and SCSI. Other buses can have additional > complications, like PCI with its multiple D states. But the principle > remains the same.) > > > Ideally it would ask the lower level for permission to do an autosuspend. > > I'd > > like to change the API so that you can do that. But I don't think that the > > lower levels have to implement autosuspend on their own to have levels > > above them support autosuspend. Can you summarize your requirements > > for supporting autosuspend in the higher levels? > > It's very simple: The higher level can't autosuspend if doing so would > cause harm to the lower level. > > There are two ways to avoid harm. One is for the lower level to be > such that it can never be harmed, no matter what the higher level does. > For example, a purely logical entity like a partition won't be harmed > if the drive it belongs to is suspended. In fact we don't try to > suspend partitions, and they don't even have drivers. > > The other way is for the lower level to be suspended already. That's > how the autosuspend framework operates: the lower level autosuspends > and tells the higher level that it is now safe for the higher level to This is how the hub driver works. > autosuspend. It's not supposed to work by the higher level announcing: > "I want to autosuspend now, so all you lower guys had better get > ready." I see. And there's an appealing simplicity to it. But why insist that this is the one true way? > Even in the case of system suspend things don't work that way. We > don't have higher-level drivers telling lower-level drivers to suspend. > Rather, the PM core (acting on behalf of the user) tells _every_ driver > to suspend -- in the correct order, of course. True. And putting the notification into a driver is a kludge at best. It simply was the only way I could come up with without moving autosuspend into generic code. Nevertheless, I am not convinced that autosuspend has to work on the device level only. > Now, how much extra work is involved in having the lower-level drivers > implement autosuspend as opposed to having the higher-level driver ask > permission? Not much more than adding the autosuspend timers. > Everything else is needed anyway for supporting manual runtime suspend. Move autosuspend into generic code and I'll certainly try to come up with something better than what I wrote. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: dc395x.c: Fix for possible null pointer dereference
On Sun, 2014-05-18 at 21:50 +0200, Guennadi Liakhovetski wrote: > On Sun, 18 May 2014, Rickard Strandqvist wrote: > > > There is otherwise a risk of a possible null pointer dereference. > > > > Was largely found by using a static code analysis program called cppcheck. > > > > Signed-off-by: Rickard Strandqvist > > --- > > drivers/scsi/dc395x.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c > > index 83d9bf6..0d86bc7 100644 > > --- a/drivers/scsi/dc395x.c > > +++ b/drivers/scsi/dc395x.c > > @@ -2637,7 +2637,7 @@ static struct ScsiReqBlk *msgin_qtag(struct > > AdapterCtlBlk *acb, > > struct ScsiReqBlk *srb = NULL; > > struct ScsiReqBlk *i; > > dprintkdbg(DBG_0, "msgin_qtag: (0x%p) tag=%i srb=%p\n", > > - srb->cmd, tag, srb); > > + srb ? srb->cmd : 0, tag, srb); > > There's not just a risk, it is a NULL-pointer dereference, so, just remove > it, e.g. like Indeed. Thank you both for catching this. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH]indistinguishable devices with broken and unbroken firmware
Hi,
there's a USB mass storage device which exists in two version. One
reports the correct size and the other does not. Apart from that they
are identical and cannot be told apart. Here's a heuristic based on the
empirical finding that drives have even sizes. I know it's ugly, but it fixes
an ugly bug.
Regards
Oliver
Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
--- a/drivers/usb/storage/unusual_devs.h2007-02-06 14:14:49.0
+0100
+++ b/drivers/usb/storage/unusual_devs.h2007-02-07 15:14:01.0
+0100
@@ -1430,7 +1430,7 @@
"DataStor",
"USB4500 FW1.04",
US_SC_DEVICE, US_PR_DEVICE, NULL,
- US_FL_FIX_CAPACITY),
+ US_FL_CAPACITY_HEURISTICS),
/* Control/Bulk transport for all SubClass values */
USUAL_DEV(US_SC_RBC, US_PR_CB, USB_US_TYPE_STOR),
--- a/drivers/usb/storage/scsiglue.c2007-02-06 14:13:49.0 +0100
+++ b/drivers/usb/storage/scsiglue.c2007-02-07 15:14:01.0 +0100
@@ -170,6 +170,12 @@
if (us->flags & US_FL_FIX_CAPACITY)
sdev->fix_capacity = 1;
+ /* A few disks have two indistinguishable version, one of
+* which reports the correct capacity and the other does not.
+* The sd driver has to guess which is the case. */
+ if (us->flags & US_FL_CAPACITY_HEURISTICS)
+ sdev->guess_capacity = 1;
+
/* Some devices report a SCSI revision level above 2 but are
* unable to handle the REPORT LUNS command (for which
* support is mandatory at level 3). Since we already have
--- a/drivers/scsi/sd.c 2007-02-06 14:14:48.0 +0100
+++ b/drivers/scsi/sd.c 2007-02-07 15:14:01.0 +0100
@@ -1273,6 +1273,13 @@
if (sdp->fix_capacity)
--sdkp->capacity;
+ /* Some devices have version which report the correct sizes
+* and others which do not. We guess size according to a heuristic
+* and err on the side of lowering the capacity. */
+ if (sdp->guess_capacity)
+ if (sdkp->capacity & 0x01) /* odd sizes are odd */
+ --sdkp->capacity;
+
got_data:
if (sector_size == 0) {
sector_size = 512;
--- a/include/linux/usb_usual.h 2007-02-06 14:14:07.0 +0100
+++ b/include/linux/usb_usual.h 2007-02-07 15:14:01.0 +0100
@@ -46,7 +46,9 @@
US_FLAG(MAX_SECTORS_64, 0x0400) \
/* Sets max_sectors to 64*/ \
US_FLAG(IGNORE_DEVICE, 0x0800) \
- /* Don't claim device */
+ /* Don't claim device */\
+ US_FLAG(CAPACITY_HEURISTICS,0x1000) \
+ /* sometimes sizes is too big */
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
--- a/include/scsi/scsi_device.h2007-02-06 14:14:57.0 +0100
+++ b/include/scsi/scsi_device.h2007-02-07 15:14:01.0 +0100
@@ -122,6 +122,7 @@
unsigned no_uld_attach:1; /* disable connecting to upper level drivers
*/
unsigned select_no_atn:1;
unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */
+ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1
*/
unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */
unsigned int device_blocked;/* Device returned QUEUE_FULL. */
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]indistinguishable devices with broken and unbroken firmware
Am Mittwoch, 7. Februar 2007 16:05 schrieb Christoph Hellwig: > On Wed, Feb 07, 2007 at 03:22:39PM +0100, Oliver Neukum wrote: > > Hi, > > > > there's a USB mass storage device which exists in two version. One > > reports the correct size and the other does not. Apart from that they > > are identical and cannot be told apart. Here's a heuristic based on the > > empirical finding that drives have even sizes. I know it's ugly, but it > > fixes > > an ugly bug. > > Do we really need a second flag? Alternatively fix_capacity could be > changed to mean fix size if odd. Until we learn about devices which really do have an odd number of sectors. And then we'll have a problem. I'd like to avoid heuristics when we know for sure. Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH]indistinguishable devices with broken and unbroken firmware #2
Hi,
there's a USB mass storage device which exists in two version. One
reports the correct size and the other does not. Apart from that they
are identical and cannot be told apart. Here's a heuristic based on the
empirical finding that drives have even sizes.
The requested changes have been made.
Greg, can this go through your tree?
Regards
Oliver
Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
--
--- linux-2.6.20/drivers/usb/storage/unusual_devs.h 2007-02-06
14:14:49.0 +0100
+++ linux-2.6.20-autosusp/drivers/usb/storage/unusual_devs.h2007-02-07
15:14:01.0 +0100
@@ -1430,7 +1430,7 @@
"DataStor",
"USB4500 FW1.04",
US_SC_DEVICE, US_PR_DEVICE, NULL,
- US_FL_FIX_CAPACITY),
+ US_FL_CAPACITY_HEURISTICS),
/* Control/Bulk transport for all SubClass values */
USUAL_DEV(US_SC_RBC, US_PR_CB, USB_US_TYPE_STOR),
--- linux-2.6.20/drivers/usb/storage/scsiglue.c 2007-02-06 14:13:49.0
+0100
+++ linux-2.6.20-autosusp/drivers/usb/storage/scsiglue.c2007-02-07
15:14:01.0 +0100
@@ -170,6 +170,12 @@
if (us->flags & US_FL_FIX_CAPACITY)
sdev->fix_capacity = 1;
+ /* A few disks have two indistinguishable version, one of
+* which reports the correct capacity and the other does not.
+* The sd driver has to guess which is the case. */
+ if (us->flags & US_FL_CAPACITY_HEURISTICS)
+ sdev->guess_capacity = 1;
+
/* Some devices report a SCSI revision level above 2 but are
* unable to handle the REPORT LUNS command (for which
* support is mandatory at level 3). Since we already have
--- linux-2.6.20/drivers/scsi/sd.c 2007-02-06 14:14:48.0 +0100
+++ linux-2.6.20-autosusp/drivers/scsi/sd.c 2007-02-08 08:49:22.0
+0100
@@ -1270,9 +1270,18 @@
/* Some devices return the total number of sectors, not the
* highest sector number. Make the necessary adjustment. */
- if (sdp->fix_capacity)
+ if (sdp->fix_capacity) {
--sdkp->capacity;
+ /* Some devices have version which report the correct sizes
+* and others which do not. We guess size according to a heuristic
+* and err on the side of lowering the capacity. */
+ } else {
+ if (sdp->guess_capacity)
+ if (sdkp->capacity & 0x01) /* odd sizes are odd */
+ --sdkp->capacity;
+ }
+
got_data:
if (sector_size == 0) {
sector_size = 512;
--- linux-2.6.20/include/linux/usb_usual.h 2007-02-06 14:14:07.0
+0100
+++ linux-2.6.20-autosusp/include/linux/usb_usual.h 2007-02-07
15:14:01.0 +0100
@@ -46,7 +46,9 @@
US_FLAG(MAX_SECTORS_64, 0x0400) \
/* Sets max_sectors to 64*/ \
US_FLAG(IGNORE_DEVICE, 0x0800) \
- /* Don't claim device */
+ /* Don't claim device */\
+ US_FLAG(CAPACITY_HEURISTICS,0x1000) \
+ /* sometimes sizes is too big */
#define US_FLAG(name, value) US_FL_##name = value ,
enum { US_DO_ALL_FLAGS };
--- linux-2.6.20/include/scsi/scsi_device.h 2007-02-06 14:14:57.0
+0100
+++ linux-2.6.20-autosusp/include/scsi/scsi_device.h2007-02-07
15:14:01.0 +0100
@@ -122,6 +122,7 @@
unsigned no_uld_attach:1; /* disable connecting to upper level drivers
*/
unsigned select_no_atn:1;
unsigned fix_capacity:1;/* READ_CAPACITY is too high by 1 */
+ unsigned guess_capacity:1; /* READ_CAPACITY might be too high by 1
*/
unsigned retry_hwerror:1; /* Retry HARDWARE_ERROR */
unsigned int device_blocked;/* Device returned QUEUE_FULL. */
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] libata: reimplement suspend/resume support using sdev->manage_start_stop
Am Dienstag, 20. März 2007 17:39 schrieb Alan Cox: > > * sdev->manage_start_stop is set to 1 in ata_scsi_slave_config(). > > This fixes spindown on shutdown and suspend-to-disk. > > Yay Which kernel version is this? Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
