Re: [PATCH v7 2/6] scsi: sr: support runtime pm
On Tuesday 25 September 2012 22:46:06 Aaron Lu wrote: > On 09/25/2012 10:23 PM, Oliver Neukum wrote: > >> 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. Hooks can be called with the necessary parameters. I suggest a triplett of medium presence, tray state and door lock state. That should cover most types of drives. > > PS: Are you sure sr_suspend() handles DVD-RAMs correctly? > > No. Is there a spec for it? Mount Fuji I presume. > Considering there are many different drives sr handle, is it possible to > write a generic sr_suspend? There are two different issues. sr handles some different devices: CD/DVD/BD-ROMs, -writers and -RAMs. For those you can have different code paths in sr. That is no problem at all. In addition devices can be attached by different hardware. In fact the same drive can be attached in a USB enclosure or by SATA. >From the perspective of power management they are no longer the same device. Those are best handled in callbacks and limited use of special cases in sr. > 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. > Does this sound reasonable? No. It would badly harm usb-storage. You need to leave paths open for other device types. 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] sd: Ensure we correctly disable devices with unknown protection type
Hi Martin, The patch fe542396da73: "[SCSI] sd: Ensure we correctly disable devices with unknown protection type" from Sep 21, 2012, leads to the following warning: include/scsi/scsi_host.h:879 scsi_host_dif_capable() warn: buffer overflow 'cap' 4 <= 4 838 enum scsi_host_prot_capabilities { 839 SHOST_DIF_TYPE1_PROTECTION = 1 << 0, /* T10 DIF Type 1 */ 840 SHOST_DIF_TYPE2_PROTECTION = 1 << 1, /* T10 DIF Type 2 */ 841 SHOST_DIF_TYPE3_PROTECTION = 1 << 2, /* T10 DIF Type 3 */ 842 843 SHOST_DIX_TYPE0_PROTECTION = 1 << 3, /* DIX between OS and HBA only */ 844 SHOST_DIX_TYPE1_PROTECTION = 1 << 4, /* DIX with DIF Type 1 */ 845 SHOST_DIX_TYPE2_PROTECTION = 1 << 5, /* DIX with DIF Type 2 */ 846 SHOST_DIX_TYPE3_PROTECTION = 1 << 6, /* DIX with DIF Type 3 */ 847 }; 869 static inline unsigned int scsi_host_dif_capable(struct Scsi_Host *shost, unsigned int target_type) 870 { 871 static unsigned char cap[] = { 0, 872 SHOST_DIF_TYPE1_PROTECTION, 873 SHOST_DIF_TYPE2_PROTECTION, 874 SHOST_DIF_TYPE3_PROTECTION }; 875 876 if (target_type > SHOST_DIF_TYPE3_PROTECTION) 877 return 0; 878 879 return shost->prot_capabilities & cap[target_type] ? target_type : 0; It looks like we're doing cap[1 << 2] here not cap[2]. 880 } 882 static inline unsigned int scsi_host_dix_capable(struct Scsi_Host *shost, unsigned int target_type) 883 { 884 #if defined(CONFIG_BLK_DEV_INTEGRITY) 885 static unsigned char cap[] = { SHOST_DIX_TYPE0_PROTECTION, 886 SHOST_DIX_TYPE1_PROTECTION, 887 SHOST_DIX_TYPE2_PROTECTION, 888 SHOST_DIX_TYPE3_PROTECTION }; 889 890 if (target_type > SHOST_DIX_TYPE3_PROTECTION) 891 return 0; 892 893 return shost->prot_capabilities & cap[target_type]; ^ We're doing cap[1 << 6] here, not cap[6]; 894 #endif 895 return 0; 896 } regards, dan carpenter -- 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 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: [usb-storage] Re: usb3 fails to write when using usb3 hub in usb3 port
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 -- 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 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_
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 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
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 wrote: > On Wed, Sep 26, 2012 at 5:50 PM, Alan Stern 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_tables
[PATCH] Scsi: Fixed white space and tab errors.
Signed-off-by: Josh Taylor --- 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: spi
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 0/5] Add new fcoe_sysfs based control interfaces to libfcoe, bnx2fc and fcoe
This series applies to the v3.5 kernel. The following series adds /sys/bus/fcoe based control interfaces to libfcoe. A fcoe_sysfs infrastructure was added to the kernel a few cycles ago, this series builds on that work. The patches deprecate the old create, vn2vn_create, destroy, enable and disable interfaces that exist in /sys/module/libfcoe/parameters/. Another goal of this series is to change the initialization sequence for a FCoE device. The result of this series is that interfaces created using libfcoe.ko interfaces (i.e. fcoe.ko or bnx2fc.ko) will have the following starting steps- 1) Create/alloc the FCoE Controller - Allocate kernel memory and create per-instance sysfs devices - The FCoE Controller is created disabled, no discovery or login until it is enabled. # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_create 2) Configure the FCoE Controller - Change mode, set ddp_min (future), set dcb_required (future), etc... # echo 2 > /sys/bus/fcoe/ctlr_0/mode #sets mode to VN2VN 3) Enable the FCoE Controller - Begins discovery and login in 'Fabric' mode. or - Begins FIP FCID claim process, discovery and login in 'VN2VN' mode 4) Destroy the FCoE Controller - Logout and free all memory # echo eth3.172-fcoe > /sys/bus/fcoe/ctlr_destroy This series converts both fcoe.ko and bnx2fc.ko to use the new fcoe_sysfs based interfaces. The legacy interfaces can remain in kernel for a kernel cycle or two before being removed. I'm looking for feedback on the use of /sys/bus/fcoe/ctlr_create and /sys/bus/fcoe/ctlr_destroy and that those interfaces take an ifname. I modeled it after the bonding interface, but I'm not sure if that's acceptible. Incorpoated v1 feedback: @Chris: I spent a lot of time looking into FCF selection. I implemented a POC series where I made the 'enabled' attribute of a fcoe_fcf_device (i.e. /sys/bus/fcoe/devices/fcf_X) writable. The fcoe_ctlr_device (i.e. /sys/bus/fcoe/devices/ctlr_X) has a 'selection_strategy' attribute that would allow the user to toggle between AUTO (current kernel selection algorithm) and USER (user writes to FCF's 'selection' attribute). I also wrote a little libudev based application that listened for fcoe_fcf_device sysfs add/remove events. My plan was to have fcoemon monitor the discovery of FCFs and then have it write to the 'selected' attribute of the FCF the user had chosen. Working on this series convinced me that any FCF selection needs further consideration. First of all, it's fairly complicated to update the fcoe_ctlr.c functional FIP code to handle FCF/fabric selection. Some questions that arise: What triggers FLOGI/LOGO? We would now have link, enabled/disabled, selection strategy and FCF selection to consider. Can a new FCF be selected when one is already selected and an FLOGI has occurred? Can a FCF be de-selected when the FLOGI has been sent? Maybe we can only change things when disabled, that probably makes the most sense.. When does discovery happen? When the ctlr is created (no opportunity for mode/selection strategy to be set)? When the mode is changed (same problem)? What about when the cltr is enabled (but that's when we were going to FLOGI)? This isn't a complete list of considerations, just what came to mind when writing this. Anyway, the policies started to get complicated, or maybe my lack of policies made the POC implementation more complicated. Either way it made me think about use cases and how valuable FCF/fabric selection is. After further consideration I think that most of the access decisions, when connecting to a FC fabric, should be done by the fabric services. I'm not sure the endstations should be whitelisting or blacklisting FCFs or even making decisions about which ones to use when they're already prioritized amongst themselves (on the same fabric). I think it might be nice for debugging, but I don't have a need at the moment. I think user selection may be more valuable with VN2VN, which may interest you more anyway, as you said that you were running a VN2VN setup. Since there aren't fabric services to rely on I can see it useful for VN_Ports to whitelist or blacklist other VN_Ports. I think the first step to support something like this would be to represent the fcoe_rports in fcoe_sysfs or in the FC Transport such that they can be selected or de-slected. I think that's a fine goal, but with this series, I think I'd like to focus on just getting away from using module parameters for control interfaces. I think this current series allows for selection as I've described above since it will now start the FCoE Controller in a DISABLED state such that configurations can now be made before the FLOGI. @Bhanu I implemented the changes for bnx2fc and I think it should be mostly fine. I wasn't able to test the code, so I'd appreciate any feedback about whether there are bugs or not. @Bart: Fixed the 'const char *p' and cast issue in fcoe_parse_mode(). Now checking for string length and
[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 --- 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 +/* + * OK to include local libfcoe.h for debug_logging, but cannot include + * 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 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 --- 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 , 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 + 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 , 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 mode at a time. + + enabled: Whether an FCoE controller is enabled
[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 --- 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 net_dev
[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 --- 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: [PATCH scsi] Add NULL checking of return value from scsi_cmd_to_driver()
> "James" == James Bottomley 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? -- Martin K. Petersen Oracle Linux Engineering -- 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
> "James" == James Bottomley 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? -- Martin K. Petersen Oracle Linux Engineering block/scsi: Allow request and error handling timeouts to be specified Export rq_timeout in sysfs for block devices since it is a request_queue property. Until now it has only been possible to explicitly set this for SCSI class devices. Also introduce eh_timeout which can be used for error handling purposes. This was previously hardcoded to 10 seconds in the SCSI error handling code. However, for some fast-fail scenarios it is necessary to be able to tune this as it can take several iterations (bus device, target, bus, controller) before we give up. Signed-off-by: Martin K. Petersen diff --git a/Documentation/ABI/testing/sysfs-block b/Documentation/ABI/testing/sysfs-block index c1eb41c..0ad8442 100644 --- a/Documentation/ABI/testing/sysfs-block +++ b/Documentation/ABI/testing/sysfs-block @@ -129,6 +129,29 @@ Description: throughput is desired. If no optimal I/O size is reported this file contains 0. +What: /sys/block//queue/rq_timeout_secs +Date: June 2012 +Contact: Martin K. Petersen +Description: + This is the timeout in seconds for regular filesystem + I/O requests. If the disk device has not completed the + request in this many seconds the kernel will fail the + I/O and initiate the subsystem-specific error handling + process. + +What: /sys/block//queue/eh_timeout_secs +Date: June 2012 +Contact: Martin K. Petersen +Description: + If a device has timed out while processing regular + filesystem I/O the kernel will attempt to bring the + device back online. This typically involves an + escalating set of approaches (device reset, bus reset, + controller reset). eh_timeout_secs describes how many + seconds should be spent waiting for response after each + recovery attempt before moving on to the next step in + the error handling process. + What: /sys/block//queue/nomerges Date: January 2010 Contact: diff --git a/block/blk-settings.c b/block/blk-settings.c index 565a678..7cc6066 100644 --- a/block/blk-settings.c +++ b/block/blk-settings.c @@ -87,6 +87,12 @@ void blk_queue_rq_timeout(struct request_queue *q, unsigned int timeout) } EXPORT_SYMBOL_GPL(blk_queue_rq_timeout); +void blk_queue_eh_timeout(struct request_queue *q, unsigned int timeout) +{ + q->eh_timeout = timeout; +} +EXPORT_SYMBOL_GPL(blk_queue_eh_timeout); + void blk_queue_rq_timed_out(struct request_queue *q, rq_timed_out_fn *fn) { q->rq_timed_out_fn = fn; diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index 9628b29..7725c84 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -280,6 +280,42 @@ queue_rq_affinity_store(struct request_queue *q, const char *page, size_t count) return ret; } +static ssize_t +queue_rq_timeout_store(struct request_queue *q, const char *page, size_t count) +{ + unsigned long rq_timeout; + ssize_t ret = queue_var_store(&rq_timeout, page, count); + + q->rq_timeout = rq_timeout * HZ; + + return ret; +} + +static ssize_t queue_rq_timeout_show(struct request_queue *q, char *page) +{ + int rq_timeout = q->rq_timeout / HZ; + + return queue_var_show(rq_timeout, (page)); +} + +static ssize_t +queue_eh_timeout_store(struct request_queue *q, const char *page, size_t count) +{ + unsigned long eh_timeout; + ssize_t ret = queue_var_store(&eh_timeout, page, count); + + q->eh_timeout = eh_timeout * HZ; + + return ret; +} + +static ssize_t queue_eh_timeout_show(struct request_queue *q, char *page) +{ + int eh_timeout = q->eh_timeout / HZ; + + return queue_var_show(eh_timeout, (page)); +} + static struct queue_sysfs_entry queue_requests_entry = { .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR }, .show = queue_requests_show, @@ -394,6 +430,18 @@ static struct queue_sysfs_entry queue_random_entry = { .store = queue_store_random, }; +static struct queue_sysfs_entry queue_rq_timeout_entry = { + .attr = {.name = "rq_timeout_secs", .mode = S_IRUGO | S_IWUSR }, + .show = queue_rq_timeout_show, + .s
Re: [SCSI] sd: Ensure we correctly disable devices with unknown protection type
> "Dan" == Dan Carpenter 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 Signed-off-by: Martin K. Petersen 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 > > 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 > > 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