Re: [PATCH v7 2/6] scsi: sr: support runtime pm
On Wednesday, September 26, 2012, Aaron Lu wrote: On Tue, Sep 25, 2012 at 11:45:34PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: On 09/25/2012 10:23 PM, Oliver Neukum wrote: On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. OK. Then these scsi bindings for sata drives will be pretty much of no use I think. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Thanks for the suggestion. The problem is, I need to know whether the door is closed and if there is a medium inside. I've no way of getting such information in libata. How does sr get to know it in the libata case? By executing a test_unit_ready command. Libata does/should not have any routine to do this, it is one of the transport of SCSI devices and it relies on SCSI driver to manage the device(both disk and ODD). PS: Are you sure sr_suspend() handles DVD-RAMs correctly? No. Is there a spec for it? Considering there are many different drives sr handle, is it possible to write a generic sr_suspend? Maybe your suggestion of callback is the way to go. What about this idea, if we find this is a ZPODD capable drive, we enable runtime suspend for it and write a suspend callback according to ZPODD spec. For other drives that does not have a suspend callback, we do not enable runtime suspend. You can enable runtime PM for all kinds of drives, but make the suspend and resume callbacks only do something for ZPODD ones. This may allow their parents to use runtime PM (as Alan said earlier in this thread), even if the drives themseleves are not really physically suspended. Sounds good. Does this sound reasonable? First, we need to know when the drive is not in use. That information we can get from the sr's runtime PM and it looks like we need to notify libata about that somehow. I'm not sure what mechanism is the best for that at the moment. The current mechanism to notify libata is by rumtime suspend. When scsi device is runtime suspended, its parent device will be suspended. And ata_port is one of the ancestor devices of scsi device, and we will remove its power in ata_port's runtime suspend callback. The problem, then, is that the ata_port's runtime suspend callback would have to know whether or not power can be removed from the drive. I'm going to post patches introducing a no power off flag for PM QoS, among other things, today or tomorrow. I suppose this flag might be used to tell the ata_port's suspend not to remove power from the attached device. Second, when the device is resumed by remote wakeup, we need to notify sr about that. A resume alone is not sufficient, though, because it may be necessary to open the tray. Perhaps in that case we can use the same mechanism by which user events are processed by libata and delivered to sr? Thanks for the suggestion. I'm not aware of any user events processed by libata. Do you mean the events_checking poll? Yes, basically. In the remote wakeup case libata might report the same status as in the user pressed the eject button situation happening normally with power on. Thanks, Rafael -- 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 v7 2/6] scsi: sr: support runtime pm
On Wed, Sep 26, 2012 at 7:18 PM, Rafael J. Wysocki r...@sisk.pl wrote: On Wednesday, September 26, 2012, Aaron Lu wrote: On Tue, Sep 25, 2012 at 11:45:34PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: On 09/25/2012 10:23 PM, Oliver Neukum wrote: On Tuesday 25 September 2012 22:20:21 Aaron Lu wrote: On Tue, Sep 25, 2012 at 01:47:52PM +0200, Rafael J. Wysocki wrote: On Tuesday, September 25, 2012, Aaron Lu wrote: I'm thinking of enabling this GPE in sr_suspend once we decided that it is ready to be powered off, so the time frame between sr_suspend and when the power is actually removed in libata should be taken care of by the GPE. If GPE fires, the notification function will request a runtime resume of the device. Does this sound OK? Well, depending on the implementation. sr_suspend() should be rather generic, but the ACPI association (including the GPE thing) is specific to ATA. Sorry, but don't quite understand this. We have ACPI bindings for scsi devices, isn't that for us to use ACPI when needed in scsi? We don't have ACPI bindings for generic SCSI devices. We have such bindings for SATA drives. You can put such things only in sr if it applies to all (maybe most) types of drives. OK. Then these scsi bindings for sata drives will be pretty much of no use I think. BTW, if sr_suspend should be generic, that would suggest I shouldn't write any ZPODD related code there, right? Any suggestion where these code should go then? libata. Maybe some generic hooks can be called in sr_suspend(). Thanks for the suggestion. The problem is, I need to know whether the door is closed and if there is a medium inside. I've no way of getting such information in libata. How does sr get to know it in the libata case? By executing a test_unit_ready command. Libata does/should not have any routine to do this, it is one of the transport of SCSI devices and it relies on SCSI driver to manage the device(both disk and ODD). PS: Are you sure sr_suspend() handles DVD-RAMs correctly? No. Is there a spec for it? Considering there are many different drives sr handle, is it possible to write a generic sr_suspend? Maybe your suggestion of callback is the way to go. What about this idea, if we find this is a ZPODD capable drive, we enable runtime suspend for it and write a suspend callback according to ZPODD spec. For other drives that does not have a suspend callback, we do not enable runtime suspend. You can enable runtime PM for all kinds of drives, but make the suspend and resume callbacks only do something for ZPODD ones. This may allow their parents to use runtime PM (as Alan said earlier in this thread), even if the drives themseleves are not really physically suspended. Sounds good. Does this sound reasonable? First, we need to know when the drive is not in use. That information we can get from the sr's runtime PM and it looks like we need to notify libata about that somehow. I'm not sure what mechanism is the best for that at the moment. The current mechanism to notify libata is by rumtime suspend. When scsi device is runtime suspended, its parent device will be suspended. And ata_port is one of the ancestor devices of scsi device, and we will remove its power in ata_port's runtime suspend callback. The problem, then, is that the ata_port's runtime suspend callback would have to know whether or not power can be removed from the drive. I'm going to post patches introducing a no power off flag for PM QoS, among other things, today or tomorrow. I suppose this flag might be used to tell the ata_port's suspend not to remove power from the attached device. Cool, thanks. Second, when the device is resumed by remote wakeup, we need to notify sr about that. A resume alone is not sufficient, though, because it may be necessary to open the tray. Perhaps in that case we can use the same mechanism by which user events are processed by libata and delivered to sr? Thanks for the suggestion. I'm not aware of any user events processed by libata. Do you mean the events_checking poll? Yes, basically. In the remote wakeup case libata might report the same status as in the user pressed the eject button situation happening normally with power on. Maybe I didn't explain it clearly. The user pressed the eject button is reported by ACPI through GPE, while the events_checking poll sends a command to the device to let it report events like media_change, etc. And the events is reported to user space, that doesn't seem can help us in this case. Thanks, Aaron -- 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: usb3 fails to write when using usb3 hub in usb3 port
On Wed, Sep 26, 2012 at 5:50 PM, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 25 Sep 2012, Sarah Sharp wrote: Alan, I'm wondering if the xHCI ring expansion is causing issues with USB hard drives under xHCI. Testing with a Buffalo USB 3.0 hard drive with an NEC uPD720200 xHCI host, I see that the usb-storage and SCSI initialization produces I/O errors on random sectors in 3.4.0, 3.4.6, and 3.5.0. I can't get those errors to be reproduced in 3.3.1. The xHCI ring expansion was added in 3.4, and we changed the xHCI's sg_tablesize: int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) { ... /* Accept arbitrarily long scatter-gather lists */ hcd-self.sg_tablesize = ~0; The usb-storage driver sets the tablesize thus: static unsigned int usb_stor_sg_tablesize(struct usb_interface *intf) { struct usb_device *usb_dev = interface_to_usbdev(intf); if (usb_dev-bus-sg_tablesize) { return usb_dev-bus-sg_tablesize; } return SG_ALL; } I notice that SG_ALL is set to SCSI_MAX_SG_SEGMENTS, which is only 128. Should we be passing an arbitrarily large number to the SCSI core? Yes, there's no reason not to. The block layer will make sure that each individual request has a sufficiently small number of segments. There's some wording in include/scsi/scsi.h about also limiting the number of chained sgs to 2048. I'm wondering if we're hitting some bugs in the SCSI layer because we're setting the sg_tablesize so high. I doubt it. Anyway, this stuff is handled by the block layer now, not the SCSI layer. If you look through drivers/scsi, you'll see that SG_ALL is used only in various SCSI interface drivers, not in the core. Alternately, we could be hitting bugs in the USB 3.0 firmware when we attempt to issue a read or write that's too big. The read on Adrian's hard drive failed on a bigger read request (122880 bytes). It would be interesting to see if it works fine if the xHCI sg_tablesize is limited. I'm going to try that with my own drive on 3.5.4 and see if it helps. There were examples in the earlier usbmon traces where 122880-byte reads succeeded, for whatever that's worth... I doubt very much that you are anywhere close to hitting that limit. If a 120-KB transfer has more than 128 SG segments then on average each segment would be under 1024 bytes, a lot smaller than a page, which seems unlikely. I don't think I've ever seen a transfer needing more than about 8 segments. Alan Stern Ok, back to vanilla 3.4.11, disabled CONFIG_USB_XHCI_HCD_DEBUGGING .. I still see 2012-09-26T19:52:16.661604+03:00 d3xt3r01 kernel: [ 1213.416759] usb 3-2.4: reset SuperSpeed USB device number 11 using xhci_hcd 2012-09-26T19:52:16.674632+03:00 d3xt3r01 kernel: [ 1213.429351] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88011d3c6980 2012-09-26T19:52:16.674665+03:00 d3xt3r01 kernel: [ 1213.429363] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88011d3c69c0 T: Bus=03 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 8 Spd=5000 MxCh= 4 D: Ver= 3.00 Cls=09(hub ) Sub=00 Prot=03 MxPS= 9 #Cfgs= 1 P: Vendor=2109 ProdID=0810 Rev= 3.74 S: Manufacturer=VIA Labs, Inc. S: Product=4-Port USB 3.0 Hub C:* #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr= 2mA I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub E: Ad=81(I) Atr=13(Int.) MxPS= 2 Ivl=4096ms T: Bus=02 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 4 Spd=480 MxCh= 4 D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=2109 ProdID=3431 Rev= 2.74 S: Product=USB2.0 Hub C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=100mA I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub E: Ad=81(I) Atr=03(Int.) MxPS= 1 Ivl=256ms T: Bus=03 Lev=02 Prnt=08 Port=00 Cnt=01 Dev#= 9 Spd=5000 MxCh= 0 D: Ver= 3.00 Cls=00(ifc ) Sub=00 Prot=00 MxPS= 9 #Cfgs= 1 P: Vendor=1058 ProdID=1140 Rev=10.03 S: Manufacturer=Western Digital S: Product=My Book 1140 S: SerialNumber=5743415A4144303235323133 C:* #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr= 2mA I:* If#= 0 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms T: Bus=03 Lev=02 Prnt=08 Port=03 Cnt=04 Dev#= 11 Spd=5000 MxCh= 0 D: Ver= 3.00 Cls=00(ifc ) Sub=00 Prot=00 MxPS= 9 #Cfgs= 1 P: Vendor=1058 ProdID=1140 Rev=10.03 S: Manufacturer=Western Digital S: Product=My Book 1140 S: SerialNumber=574D415A4135343330323937 C:* #Ifs= 1 Cfg#= 1 Atr=c0 MxPwr= 2mA I:* If#= 0 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage E: Ad=81(I) Atr=02(Bulk) MxPS=1024 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS=1024 Ivl=0ms So I need to cat 3u .. right ? Available at http://d3xt3r01.tk/~dexter/usbmon/1348678668_3u After the copy .. I see 2012-09-26T19:52:51.477641+03:00 d3xt3r01 kernel: [ 1248.232213] hub 3-2:1.0: Cannot enable port 4. Maybe the USB cable is bad?
Re: [usb-storage] Re: usb3 fails to write when using usb3 hub in usb3 port
2012-09-26T20:13:09.700606+03:00 d3xt3r01 kernel: [ 2466.455403] usb 3-1.1: reset SuperSpeed USB device number 14 using xhci_hcd 2012-09-26T20:13:09.713629+03:00 d3xt3r01 kernel: [ 2466.468373] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88011aea1300 2012-09-26T20:13:09.713667+03:00 d3xt3r01 kernel: [ 2466.468384] xhci_hcd :04:00.0: xHCI xhci_drop_endpoint called with disabled ep 88011aea1340 2012-09-26T20:13:44.480669+03:00 d3xt3r01 kernel: [ 2501.235365] hub 3-1:1.0: Cannot enable port 1. Maybe the USB cable is bad? 2012-09-26T20:13:45.005111+03:00 d3xt3r01 kernel: [ 2501.759279] sd 18:0:0:0: Device offlined - not ready after error recovery 2012-09-26T20:13:45.005118+03:00 d3xt3r01 kernel: [ 2501.759300] sd 18:0:0:0: [sdb] Unhandled error code 2012-09-26T20:13:45.005125+03:00 d3xt3r01 kernel: [ 2501.759305] sd 18:0:0:0: [sdb] Result: hostbyte=DID_ABORT driverbyte=DRIVER_OK 2012-09-26T20:13:45.005133+03:00 d3xt3r01 kernel: [ 2501.759312] sd 18:0:0:0: [sdb] CDB: Read(10): 28 00 00 03 2c 00 00 00 f0 00 2012-09-26T20:13:45.005139+03:00 d3xt3r01 kernel: [ 2501.759328] end_request: I/O error, dev sdb, sector 207872 2012-09-26T20:13:45.005146+03:00 d3xt3r01 kernel: [ 2501.759334] quiet_error: 23 callbacks suppressed 2012-09-26T20:13:45.005151+03:00 d3xt3r01 kernel: [ 2501.759339] Buffer I/O error on device sdb, logical block 25984 2012-09-26T20:13:45.005157+03:00 d3xt3r01 kernel: [ 2501.759351] Buffer I/O error on device sdb, logical block 25985 2012-09-26T20:13:45.005163+03:00 d3xt3r01 kernel: [ 2501.759357] Buffer I/O error on device sdb, logical block 25986 2012-09-26T20:13:45.005169+03:00 d3xt3r01 kernel: [ 2501.759363] Buffer I/O error on device sdb, logical block 25987 The result of a simple d3xt3r01 ~ # hdparm -tT /dev/sdb /dev/sdb: Timing cached reads: 1494 MB in 2.00 seconds = 747.20 MB/sec Timing buffered disk reads: read(2097152) returned 1572864 bytes BLKFLSBUF failed: No such device It didn't fail the first time though .. I just executed the same command twice .. Assuming that 3u is the good thing to cat .. http://d3xt3r01.tk/~dexter/usbmon/1348679651_3u .. ( given the output in /sys/kernel/debug/usb/devices ) Hope this helps more to identify the issue .. 2012-09-26T20:13:45.005175+03:00 d3xt3r01 kernel: [ 2501.759369] Buffer I/O error on device sdb, logical block 25988 2012-09-26T20:13:45.005182+03:00 d3xt3r01 kernel: [ 2501.759375] Buffer I/O error on device sdb, logical block 25989 2012-09-26T20:13:45.005188+03:00 d3xt3r01 kernel: [ 2501.759382] Buffer I/O error on device sdb, logical block 25990 2012-09-26T20:13:45.005194+03:00 d3xt3r01 kernel: [ 2501.759393] sd 18:0:0:0: rejecting I/O to offline device 2012-09-26T20:13:45.005200+03:00 d3xt3r01 kernel: [ 2501.759407] sd 18:0:0:0: [sdb] killing request 2012-09-26T20:13:45.005207+03:00 d3xt3r01 kernel: [ 2501.759415] Buffer I/O error on device sdb, logical block 25991 2012-09-26T20:13:45.005213+03:00 d3xt3r01 kernel: [ 2501.759426] Buffer I/O error on device sdb, logical block 25992 2012-09-26T20:13:45.005220+03:00 d3xt3r01 kernel: [ 2501.759432] Buffer I/O error on device sdb, logical block 25993 2012-09-26T20:13:45.005227+03:00 d3xt3r01 kernel: [ 2501.759440] sd 18:0:0:0: rejecting I/O to offline device 2012-09-26T20:13:45.005233+03:00 d3xt3r01 kernel: [ 2501.759494] sd 18:0:0:0: rejecting I/O to offline device 2012-09-26T20:13:45.005240+03:00 d3xt3r01 kernel: [ 2501.759505] sd 18:0:0:0: rejecting I/O to offline device 2012-09-26T20:13:45.005246+03:00 d3xt3r01 kernel: [ 2501.759624] sd 18:0:0:0: [sdb] Unhandled error code 2012-09-26T20:13:45.005252+03:00 d3xt3r01 kernel: [ 2501.759630] sd 18:0:0:0: [sdb] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK 2012-09-26T20:13:45.005259+03:00 d3xt3r01 kernel: [ 2501.759637] sd 18:0:0:0: [sdb] CDB: Read(10): 28 00 00 03 2c f0 00 00 10 00 2012-09-26T20:13:45.005267+03:00 d3xt3r01 kernel: [ 2501.759654] end_request: I/O error, dev sdb, sector 208112 2012-09-26T20:13:45.020696+03:00 d3xt3r01 kernel: [ 2501.775198] usb 3-1.1: USB disconnect, device number 14 On Wed, Sep 26, 2012 at 7:59 PM, Adrian Sandu dex...@d3xt3r01.tk wrote: On Wed, Sep 26, 2012 at 5:50 PM, Alan Stern st...@rowland.harvard.edu wrote: On Tue, 25 Sep 2012, Sarah Sharp wrote: Alan, I'm wondering if the xHCI ring expansion is causing issues with USB hard drives under xHCI. Testing with a Buffalo USB 3.0 hard drive with an NEC uPD720200 xHCI host, I see that the usb-storage and SCSI initialization produces I/O errors on random sectors in 3.4.0, 3.4.6, and 3.5.0. I can't get those errors to be reproduced in 3.3.1. The xHCI ring expansion was added in 3.4, and we changed the xHCI's sg_tablesize: int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks) { ... /* Accept arbitrarily long scatter-gather lists */ hcd-self.sg_tablesize = ~0; The usb-storage driver sets the tablesize thus: static unsigned int usb_stor_sg_tablesize(struct
[PATCH] Scsi: Fixed white space and tab errors.
Signed-off-by: Josh Taylor joshua.tayl...@gmail.com --- drivers/scsi/scsi.c | 66 +-- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 2936b44..00aded9 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -126,7 +126,7 @@ static const char *const scsi_device_types[] = { * @type: type number to look up */ -const char * scsi_device_type(unsigned type) +const char *scsi_device_type(unsigned type) { if (type == 0x1e) return Well-known LUN ; @@ -609,7 +609,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) printk(FAILED\n); break; case TIMEOUT_ERROR: - /* + /* * If called via scsi_times_out. */ printk(TIMEOUT\n); @@ -642,7 +642,7 @@ void scsi_log_completion(struct scsi_cmnd *cmd, int disposition) void scsi_cmd_get_serial(struct Scsi_Host *host, struct scsi_cmnd *cmd) { cmd-serial_number = host-cmd_serial_number++; - if (cmd-serial_number == 0) + if (cmd-serial_number == 0) cmd-serial_number = host-cmd_serial_number++; } EXPORT_SYMBOL(scsi_cmd_get_serial); @@ -675,7 +675,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) /* Check to see if the scsi lld made this device blocked. */ if (unlikely(scsi_device_blocked(cmd-device))) { - /* + /* * in blocked state, the command is just put back on * the device queue. The suspend state has already * blocked the queue so future requests should not @@ -694,7 +694,7 @@ int scsi_dispatch_cmd(struct scsi_cmnd *cmd) goto out; } - /* + /* * If SCSI-2 or lower, store the LUN value in cmnd. */ if (cmd-device-scsi_level = SCSI_2 @@ -805,17 +805,17 @@ void scsi_finish_command(struct scsi_cmnd *cmd) scsi_device_unbusy(sdev); -/* - * Clear the flags which say that the device/host is no longer - * capable of accepting new commands. These are set in scsi_queue.c - * for both the queue full condition on a device, and for a - * host full condition on the host. + /* +* Clear the flags which say that the device/host is no longer +* capable of accepting new commands. These are set in scsi_queue.c +* for both the queue full condition on a device, and for a +* host full condition on the host * * XXX(hch): What about locking? - */ -shost-host_blocked = 0; +*/ + shost-host_blocked = 0; starget-target_blocked = 0; -sdev-device_blocked = 0; + sdev-device_blocked = 0; /* * If we have valid sense information, then some kind of recovery @@ -829,7 +829,7 @@ void scsi_finish_command(struct scsi_cmnd *cmd) (result %x)\n, cmd-result)); good_bytes = scsi_bufflen(cmd); -if (cmd-request-cmd_type != REQ_TYPE_BLOCK_PC) { + if (cmd-request-cmd_type != REQ_TYPE_BLOCK_PC) { int old_good_bytes = good_bytes; drv = scsi_cmd_to_driver(cmd); if (drv-done) @@ -894,22 +894,22 @@ void scsi_adjust_queue_depth(struct scsi_device *sdev, int tagged, int tags) sdev-queue_depth = tags; switch (tagged) { - case MSG_ORDERED_TAG: - sdev-ordered_tags = 1; - sdev-simple_tags = 1; - break; - case MSG_SIMPLE_TAG: - sdev-ordered_tags = 0; - sdev-simple_tags = 1; - break; - default: - sdev_printk(KERN_WARNING, sdev, - scsi_adjust_queue_depth, bad queue type, - disabled\n); - case 0: - sdev-ordered_tags = sdev-simple_tags = 0; - sdev-queue_depth = tags; - break; + case MSG_ORDERED_TAG: + sdev-ordered_tags = 1; + sdev-simple_tags = 1; + break; + case MSG_SIMPLE_TAG: + sdev-ordered_tags = 0; + sdev-simple_tags = 1; + break; + default: + sdev_printk(KERN_WARNING, sdev, + scsi_adjust_queue_depth, bad queue type, + disabled\n); + case 0: + sdev-ordered_tags = sdev-simple_tags = 0; + sdev-queue_depth = tags; + break; } out:
Re: [PATCH 00/20, v4] Make ib_srp better suited for H.A. purposes
On Tue, 2012-09-25 at 17:05 +0200, Bart Van Assche wrote: On 08/09/12 17:41, Bart Van Assche wrote: [ ... ] Hello Dave, More than six weeks have elapsed since I posted version four of this patch series. It would be appreciated if you could tell me when review comments for this patch series will be posted. I'd also like to remind you that some time ago you asked other people to wait with posting more ib_srp patches until this patch series is upstream [1, 2]. Yes, it has taken me far more time than I expected to get to these. I am in the middle of fiscal-year-end thrash, and will attend to the SRP backlog next week. -- Dave Dillow National Center for Computational Science Oak Ridge National Laboratory (865) 241-6602 office -- 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
[RFC PATCH v2 1/5] fix_section_mismatch
Already fixed upstream. --- drivers/scsi/fcoe/fcoe_transport.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c index b46f43d..71cc909 100644 --- a/drivers/scsi/fcoe/fcoe_transport.c +++ b/drivers/scsi/fcoe/fcoe_transport.c @@ -502,7 +502,7 @@ static int __init fcoe_transport_init(void) return 0; } -static int __exit fcoe_transport_exit(void) +static int fcoe_transport_exit(void) { struct fcoe_transport *ft; -- 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
[RFC PATCH v2 2/5] libfcoe: Add fcoe_sysfs debug logging level
Add a macro to print fcoe_sysfs debug statements. Signed-off-by: Robert Love robert.w.l...@intel.com --- drivers/scsi/fcoe/fcoe_sysfs.c |7 +++ drivers/scsi/fcoe/libfcoe.h| 11 --- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c index 2bc1631..f207bbd 100644 --- a/drivers/scsi/fcoe/fcoe_sysfs.c +++ b/drivers/scsi/fcoe/fcoe_sysfs.c @@ -24,6 +24,13 @@ #include scsi/fcoe_sysfs.h +/* + * OK to include local libfcoe.h for debug_logging, but cannot include + * scsi/libfcoe.h otherwise non-netdev based fcoe solutions would have + * have to include more than fcoe_sysfs.h. + */ +#include libfcoe.h + static atomic_t ctlr_num; static atomic_t fcf_num; diff --git a/drivers/scsi/fcoe/libfcoe.h b/drivers/scsi/fcoe/libfcoe.h index 3a758c2..d3bb16d 100644 --- a/drivers/scsi/fcoe/libfcoe.h +++ b/drivers/scsi/fcoe/libfcoe.h @@ -2,9 +2,10 @@ #define _FCOE_LIBFCOE_H_ extern unsigned int libfcoe_debug_logging; -#define LIBFCOE_LOGGING0x01 /* General logging, not categorized */ -#define LIBFCOE_FIP_LOGGING 0x02 /* FIP logging */ -#define LIBFCOE_TRANSPORT_LOGGING 0x04 /* FCoE transport logging */ +#define LIBFCOE_LOGGING 0x01 /* General logging, not categorized */ +#define LIBFCOE_FIP_LOGGING 0x02 /* FIP logging */ +#define LIBFCOE_TRANSPORT_LOGGING 0x04 /* FCoE transport logging */ +#define LIBFCOE_SYSFS_LOGGING 0x08 /* fcoe_sysfs logging */ #define LIBFCOE_CHECK_LOGGING(LEVEL, CMD) \ do { \ @@ -27,4 +28,8 @@ do { \ LIBFCOE_CHECK_LOGGING(LIBFCOE_TRANSPORT_LOGGING,\ pr_info(%s: fmt, __func__, ##args);) +#define LIBFCOE_SYSFS_DBG(cdev, fmt, args...) \ + LIBFCOE_CHECK_LOGGING(LIBFCOE_SYSFS_LOGGING,\ + pr_info(ctlr_%d: fmt, cdev-id, ##args);) + #endif /* _FCOE_LIBFCOE_H_ */ -- 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
[RFC PATCH v2 3/5] libfcoe, fcoe, bnx2fc: Add new fcoe control interface
This patch does a few things. 1) Makes /sys/bus/fcoe/ctlr_{create,destroy} interfaces. These interfaces take an ifname and will either create an FCoE Controller or destroy an FCoE Controller depending on which file is written to. The new FCoE Controller will start in a DISABLED state and will not do discovery or login until it is ENABLED. This pause will allow us to configure the FCoE Controller before enabling it. 2) Makes the 'mode' attribute of a fcoe_ctlr_device writale. This allows the user to configure the mode in which the FCoE Controller will start in when it is ENABLED. Possible modes are 'Fabric', or 'VN2VN'. The default mode for a fcoe_ctlr{,_device} is 'Fabric'. Drivers must implement the set_fcoe_ctlr_mode routine to support this feature. libfcoe offers an exported routine to set a FCoE Controller's mode. The mode can only be changed when the FCoE Controller is DISABLED. This patch also removes the get_fcoe_ctlr_mode pointer in the fcoe_sysfs function template, the code in fcoe_ctlr.c to get the mode and the assignment of the fcoe_sysfs function pointer to the fcoe_ctlr.c implementation (in fcoe and bnx2fc). fcoe_sysfs can return that value for the mode without consulting the LLD. 3) Make a 'enabled' attribute of a fcoe_ctlr_device. On a read, fcoe_sysfs will return the attribute's value. On a write, fcoe_sysfs will call the LLD (if there is a callback) to notifiy that the enalbed state has changed. This patch maintains the old FCoE control interfaces as module parameters, but it adds comments pointing out that the old interfaces are deprecated. Signed-off-by: Robert Love robert.w.l...@intel.com --- Documentation/ABI/testing/sysfs-bus-fcoe | 42 ++ drivers/scsi/bnx2fc/bnx2fc_fcoe.c|1 drivers/scsi/fcoe/fcoe.c |1 drivers/scsi/fcoe/fcoe_sysfs.c | 199 +++--- drivers/scsi/fcoe/fcoe_transport.c | 107 include/scsi/fcoe_sysfs.h| 11 ++ include/scsi/libfcoe.h | 16 ++ 7 files changed, 352 insertions(+), 25 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe b/Documentation/ABI/testing/sysfs-bus-fcoe index 469d09c..a57bf37 100644 --- a/Documentation/ABI/testing/sysfs-bus-fcoe +++ b/Documentation/ABI/testing/sysfs-bus-fcoe @@ -1,14 +1,54 @@ +What: /sys/bus/fcoe/ +Date: August 2012 +KernelVersion: TBD +Contact: Robert Love robert.w.l...@intel.com, de...@open-fcoe.org +Description: The FCoE bus. Attributes in this directory are control interfaces. +Attributes: + + ctlr_create: 'FCoE Controller' instance creation interface. Writing an +ifname to this file will allocate and populate sysfs with a +fcoe_ctlr_device (ctlr_X). The user can then configure any +per-port settings and finally write to the fcoe_ctlr_device's +'start' attribute to begin the kernel's discovery and login +process. + + ctlr_destroy: 'FCoE Controller' instance removal interface. Writing a + fcoe_ctlr_device's sysfs name to this file will log the + fcoe_ctlr_device out of the fabric or otherwise connected + FCoE devices. It will also free all kernel memory allocated + for this fcoe_ctlr_device and any structures associated + with it, this includes the scsi_host. + What: /sys/bus/fcoe/ctlr_X Date: March 2012 KernelVersion: TBD Contact: Robert Love robert.w.l...@intel.com, de...@open-fcoe.org -Description: 'FCoE Controller' instances on the fcoe bus +Description: 'FCoE Controller' instances on the fcoe bus. + + The FCoE Controller now has a three stage creation process. + 1) Write interface name to ctlr_create 2) Configure the FCoE + Controller (ctlr_X) 3) Enable the FCoE Controller to begin + discovery and login. The FCoE Controller is destroyed by + writing it's name, i.e. ctlr_X to the ctlr_delete file. + Attributes: fcf_dev_loss_tmo: Device loss timeout peroid (see below). Changing this value will change the dev_loss_tmo for all FCFs discovered by this controller. + mode: Display or change the FCoE Controller's mode. Possible + modes are 'Fabric' and 'VN2VN'. If a FCoE Controller + is started in 'Fabric' mode then FIP FCF discovery is + initiated and ultimately a fabric login is attempted. + If a FCoE Controller is started in 'VN2VN' mode then + FIP VN2VN discovery and login is performed. A FCoE + Controller only supports one
[RFC PATCH v2 4/5] fcoe: Use the fcoe_sysfs control interface
This patch adds support for the new fcoe_sysfs control interface to fcoe.ko. It keeps the deprecated interface in tact and therefore either the legacy or the new control interfaces can be used. A mixed mode is not supported. A user must either use the new interfaces or the old ones, but not both. The fcoe_ctlr's link state is now driven by both the netdev link state as well as the fcoe_ctlr_device's enabled attribute. The link must be up and the fcoe_ctlr_device must be enabled before the FCoE Controller starts discovery or login. Signed-off-by: Robert Love robert.w.l...@intel.com --- drivers/scsi/fcoe/fcoe.c | 142 + drivers/scsi/fcoe/fcoe_ctlr.c | 17 ++--- 2 files changed, 136 insertions(+), 23 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c index e5cfd76..d6632a8 100644 --- a/drivers/scsi/fcoe/fcoe.c +++ b/drivers/scsi/fcoe/fcoe.c @@ -117,6 +117,11 @@ static int fcoe_destroy(struct net_device *netdev); static int fcoe_enable(struct net_device *netdev); static int fcoe_disable(struct net_device *netdev); +/* fcoe_syfs control interface handlers */ +static int fcoe_ctlr_alloc(struct net_device *netdev); +static int fcoe_ctlr_enabled(struct fcoe_ctlr_device *cdev); + + static struct fc_seq *fcoe_elsct_send(struct fc_lport *, u32 did, struct fc_frame *, unsigned int op, @@ -155,6 +160,8 @@ static void fcoe_ctlr_get_lesb(struct fcoe_ctlr_device *); static void fcoe_fcf_get_vlan_id(struct fcoe_fcf_device *); static struct fcoe_sysfs_function_template fcoe_sysfs_templ = { + .set_fcoe_ctlr_mode = fcoe_ctlr_set_fip_mode, + .set_fcoe_ctlr_enabled = fcoe_ctlr_enabled, .get_fcoe_ctlr_link_fail = fcoe_ctlr_get_lesb, .get_fcoe_ctlr_vlink_fail = fcoe_ctlr_get_lesb, .get_fcoe_ctlr_miss_fka = fcoe_ctlr_get_lesb, @@ -1964,6 +1971,7 @@ static int fcoe_dcb_app_notification(struct notifier_block *notifier, static int fcoe_device_notification(struct notifier_block *notifier, ulong event, void *ptr) { + struct fcoe_ctlr_device *cdev; struct fc_lport *lport = NULL; struct net_device *netdev = ptr; struct fcoe_ctlr *ctlr; @@ -2020,13 +2028,29 @@ static int fcoe_device_notification(struct notifier_block *notifier, fcoe_link_speed_update(lport); - if (link_possible !fcoe_link_ok(lport)) - fcoe_ctlr_link_up(ctlr); - else if (fcoe_ctlr_link_down(ctlr)) { - stats = per_cpu_ptr(lport-dev_stats, get_cpu()); - stats-LinkFailureCount++; - put_cpu(); - fcoe_clean_pending_queue(lport); + cdev = fcoe_ctlr_to_ctlr_dev(ctlr); + + if (link_possible !fcoe_link_ok(lport)) { + switch (cdev-enabled) { + case FCOE_CTLR_DISABLED: + pr_info(Link up while interface is disabled.\n); + break; + case FCOE_CTLR_ENABLED: + case FCOE_CTLR_UNUSED: + fcoe_ctlr_link_up(ctlr); + }; + } else if (fcoe_ctlr_link_down(ctlr)) { + switch (cdev-enabled) { + case FCOE_CTLR_DISABLED: + pr_info(Link down while interface is disabled.\n); + break; + case FCOE_CTLR_ENABLED: + case FCOE_CTLR_UNUSED: + stats = per_cpu_ptr(lport-dev_stats, get_cpu()); + stats-LinkFailureCount++; + put_cpu(); + fcoe_clean_pending_queue(lport); + }; } out: return rc; @@ -2039,6 +2063,8 @@ out: * Called from fcoe transport. * * Returns: 0 for success + * + * Deprecated: use fcoe_ctlr_enabled() */ static int fcoe_disable(struct net_device *netdev) { @@ -2098,6 +2124,33 @@ out: } /** + * fcoe_ctlr_enabled() - Enable or disable an FCoE Controller + * @cdev: The FCoE Controller that is being enabled or disabled + * + * fcoe_sysfs will ensure that the state of 'enabled' has + * changed, so no checking is necessary here. This routine simply + * calls fcoe_enable or fcoe_disable, both of which are deprecated. + * When those routines are removed the functionality can be merged + * here. + */ +static int fcoe_ctlr_enabled(struct fcoe_ctlr_device *cdev) +{ + struct fcoe_ctlr *ctlr = fcoe_ctlr_device_priv(cdev); + struct fc_lport *lport = ctlr-lp; + struct net_device *netdev = fcoe_netdev(lport); + + switch (cdev-enabled) { + case FCOE_CTLR_ENABLED: + return fcoe_enable(netdev); + case FCOE_CTLR_DISABLED: + return fcoe_disable(netdev); + case FCOE_CTLR_UNUSED: + default: + return -ENOTSUPP; + }; +} + +/** * fcoe_destroy() - Destroy a FCoE interface * @netdev : The
[RFC PATCH v2 5/5] bnx2fc: Use the fcoe_sysfs control interface
This patch adds support for the new fcoe_sysfs control interface to bnx2fc.ko. It keeps the deprecated interface in tact and therefore either the legacy or the new control interfaces can be used. A mixed mode is not supported. A user must either use the new interfaces or the old ones, but not both. The fcoe_ctlr's link state is now driven by both the netdev link state as well as the fcoe_ctlr_device's enabled attribute. The link must be up and the fcoe_ctlr_device must be enabled before the FCoE Controller starts discovery or login. Signed-off-by: Robert Love robert.w.l...@intel.com --- drivers/scsi/bnx2fc/bnx2fc_fcoe.c | 163 +++-- 1 file changed, 135 insertions(+), 28 deletions(-) diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c index dfab32d..60baf88 100644 --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c @@ -62,6 +62,10 @@ static int bnx2fc_destroy(struct net_device *net_device); static int bnx2fc_enable(struct net_device *netdev); static int bnx2fc_disable(struct net_device *netdev); +/* fcoe_syfs control interface handlers */ +static int bnx2fc_ctlr_alloc(struct net_device *netdev); +static int bnx2fc_ctlr_enabled(struct fcoe_ctlr_device *cdev); + static void bnx2fc_recv_frame(struct sk_buff *skb); static void bnx2fc_start_disc(struct bnx2fc_interface *interface); @@ -864,6 +868,7 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event, u16 vlan_id) { struct bnx2fc_hba *hba = (struct bnx2fc_hba *)context; + struct fcoe_ctlr_device *cdev; struct fc_lport *lport; struct fc_lport *vport; struct bnx2fc_interface *interface, *tmp; @@ -925,28 +930,45 @@ static void bnx2fc_indicate_netevent(void *context, unsigned long event, bnx2fc_link_speed_update(lport); + cdev = fcoe_ctlr_to_ctlr_dev(ctlr); + if (link_possible !bnx2fc_link_ok(lport)) { - /* Reset max recv frame size to default */ - fc_set_mfs(lport, BNX2FC_MFS); - /* -* ctlr link up will only be handled during -* enable to avoid sending discovery solicitation -* on a stale vlan -*/ - if (interface-enabled) - fcoe_ctlr_link_up(ctlr); + switch (cdev-enabled) { + case FCOE_CTLR_DISABLED: + pr_info(Link up while interface is disabled.\n); + break; + case FCOE_CTLR_ENABLED: + case FCOE_CTLR_UNUSED: + /* Reset max recv frame size to default */ + fc_set_mfs(lport, BNX2FC_MFS); + /* +* ctlr link up will only be handled during +* enable to avoid sending discovery +* solicitation on a stale vlan +*/ + if (interface-enabled) + fcoe_ctlr_link_up(ctlr); + }; } else if (fcoe_ctlr_link_down(ctlr)) { - mutex_lock(lport-lp_mutex); - list_for_each_entry(vport, lport-vports, list) - fc_host_port_type(vport-host) = - FC_PORTTYPE_UNKNOWN; - mutex_unlock(lport-lp_mutex); - fc_host_port_type(lport-host) = FC_PORTTYPE_UNKNOWN; - per_cpu_ptr(lport-dev_stats, - get_cpu())-LinkFailureCount++; - put_cpu(); - fcoe_clean_pending_queue(lport); - wait_for_upload = 1; + switch (cdev-enabled) { + case FCOE_CTLR_DISABLED: + pr_info(Link down while interface is disabled.\n); + break; + case FCOE_CTLR_ENABLED: + case FCOE_CTLR_UNUSED: + mutex_lock(lport-lp_mutex); + list_for_each_entry(vport, lport-vports, list) + fc_host_port_type(vport-host) = + FC_PORTTYPE_UNKNOWN; + mutex_unlock(lport-lp_mutex); + fc_host_port_type(lport-host) = + FC_PORTTYPE_UNKNOWN; + per_cpu_ptr(lport-dev_stats, + get_cpu())-LinkFailureCount++; +
Re: [SCSI] sd: Ensure we correctly disable devices with unknown protection type
Dan == Dan Carpenter dan.carpen...@oracle.com writes: Dan, Dan warn: buffer overflow 'cap' 4 = 4 Argh, yes. Type 3 is 4 because it's a bitmask. -- Martin K. Petersen Oracle Linux Engineering SCSI: Fix range check in scsi_host.h The range checking from fe542396 was bad. We would still end up walking beyond the array as Type 3 is defined to be 4 in the protection bitmask. Instead use ARRAY_SIZE() for the range check. Reported-by: Dan Carpenter dan.carpen...@oracle.com Signed-off-by: Martin K. Petersen martin.peter...@oracle.com diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 4908480..2b6956e 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -873,7 +873,7 @@ static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsign SHOST_DIF_TYPE2_PROTECTION, SHOST_DIF_TYPE3_PROTECTION }; - if (target_type SHOST_DIF_TYPE3_PROTECTION) + if (target_type = ARRAY_SIZE(cap)) return 0; return shost-prot_capabilities cap[target_type] ? target_type : 0; @@ -887,7 +887,7 @@ static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsign SHOST_DIX_TYPE2_PROTECTION, SHOST_DIX_TYPE3_PROTECTION }; - if (target_type SHOST_DIX_TYPE3_PROTECTION) + if (target_type = ARRAY_SIZE(cap)) return 0; return shost-prot_capabilities cap[target_type]; -- 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] Add NULL checking of return value from scsi_cmd_to_driver()
On Wed, 2012-09-26 at 22:02 -0400, Martin K. Petersen wrote: James == James Bottomley james.bottom...@hansenpartnership.com writes: James Plus, I think it fixes a bug where you get different behaviours James from REQ_TYPE_BLOCK_PC commands when a driver is and isn't James attached (I've cc'd Martin to see what he thinks). I'm fine with having the eh action be triggered for FS requests only, if that's what you're asking? Sort of ... I was thinking do it for all non REQ_TYPE_BLOCK_PC commands (which includes flush and other strange types), but if you think it should only be for REQ_TYPE_FS, that's fine too. James -- 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 PATCH] sd: max-retries becomes configurable
On Wed, 2012-09-26 at 22:20 -0400, Martin K. Petersen wrote: James == James Bottomley james.bottom...@hansenpartnership.com writes: James On Mon, 2012-09-24 at 17:00 -0400, Jeff Garzik wrote: drivers/scsi/sd.c | 4 drivers/scsi/sd.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) James I'm not opposed in principle to doing this (except that it should James be a sysfs parameter like all our other controls), Now that we're in that department. I never got any feedback on the following patch. Hannes told me in person that he felt the eh_timeout belonged in scsi_device and not in the request queue. Whereas I favored making it a block layer tunable despite currently only being used by SCSI. Any opinions? request_queue makes more sense to me because there was once a plan to move all our timeout processing to block. I think it got stalled somewhere, but this would act as a reminder. James -- 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 PATCH] sd: max-retries becomes configurable
On 09/25/2012 06:38 AM, James Bottomley wrote: On Tue, 2012-09-25 at 01:21 -0400, Jeff Garzik wrote: Can you be more specific about sysfs location? A runtime-writable (via sysfs!) module parameter for a module-wide default seemed appropriate. Well, if it's really important, the same thing should happen with retries as happened with timeout (it became a request_queue property), but it could be hacked as a struct scsi_disk one with a corresponding entry in sd_dis_attrs. Well, it is already a request property... but assigned at initialization from sd-specific code. sd also passes this through scmd-allowed to rq-retries. It could become a request_queue property, but that seems like a hack as it is simply passed right back into SCSI EH, for SCSI-specific disposition. Jeff -- 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