Re: [Xen-devel] [PATCH 1/4] Add XEN pvSCSI protocol description
On 06/27/2014 07:11 PM, Konrad Rzeszutek Wilk wrote: On Fri, Jun 27, 2014 at 04:34:33PM +0200, jgr...@suse.com wrote: From: Juergen Gross jgr...@suse.com Add the definition of pvSCSI protocol used between the pvSCSI frontend in a XEN domU and the pvSCSI backend in a XEN driver domain (usually Dom0). This header was originally provided by Fujitsu for XEN based on Linux 2.6.18. Changes are: - added comment - adapt to Linux style guide Signed-off-by: Juergen Gross jgr...@suse.com --- include/xen/interface/io/vscsiif.h | 110 + 1 file changed, 110 insertions(+) create mode 100644 include/xen/interface/io/vscsiif.h diff --git a/include/xen/interface/io/vscsiif.h b/include/xen/interface/io/vscsiif.h new file mode 100644 index 000..f8420f3 --- /dev/null +++ b/include/xen/interface/io/vscsiif.h @@ -0,0 +1,110 @@ +/** + * vscsiif.h + * + * Based on the blkif.h code. + * + * This interface is to be regarded as a stable API between XEN domains + * running potentially different Linux kernel versions. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Copyright(c) FUJITSU Limited 2008. + */ + +#ifndef __XEN__PUBLIC_IO_SCSI_H__ +#define __XEN__PUBLIC_IO_SCSI_H__ + +#include ring.h +#include ../grant_table.h + +/* commands between backend and frontend */ +#define VSCSIIF_ACT_SCSI_CDB 1 /* SCSI CDB command */ +#define VSCSIIF_ACT_SCSI_ABORT 2 /* SCSI Device(Lun) Abort*/ +#define VSCSIIF_ACT_SCSI_RESET 3 /* SCSI Device(Lun) Reset*/ +#define VSCSIIF_ACT_SCSI_SG_PRESET 4 /* Preset SG elements */ + +/* + * Maximum scatter/gather segments per request. + * + * Considering balance between allocating at least 16 vscsiif_request + * structures on one page (4096 bytes) and the number of scatter/gather + * elements needed, we decided to use 26 as a magic number. + */ +#define VSCSIIF_SG_TABLESIZE 26 + +/* + * based on Linux kernel 2.6.18 This being a bit more .. new, - do these sizes make sense anymore? Should they be extended a bit? Or have support for using the old ones (as default) and then negotiate new sizes with the kernel? (If of course there is a difference?) The sizes are still the same. Additionally the sizes can't be changed without breaking the interface (the request structure must not change size). Added a comment. + */ +#define VSCSIIF_MAX_COMMAND_SIZE 16 +#define VSCSIIF_SENSE_BUFFERSIZE 96 + +struct scsiif_request_segment { + grant_ref_t gref; + uint16_t offset; + uint16_t length; +}; +typedef struct scsiif_request_segment vscsiif_segment_t; No typedefs please. Okay. + +struct vscsiif_request { + uint16_t rqid; /* private guest value, echoed in resp */ + uint8_t act;/* command between backend and frontend */ + uint8_t cmd_len; + + uint8_t cmnd[VSCSIIF_MAX_COMMAND_SIZE]; + uint16_t timeout_per_command; /* The command is issued by twice + the value in Backend. */ + uint16_t channel, id, lun; + uint16_t padding; + uint8_t sc_data_direction; /* for DMA_TO_DEVICE(1) + DMA_FROM_DEVICE(2) + DMA_NONE(3) requests */ + uint8_t nr_segments;/* Number of pieces of scatter-gather */ + + vscsiif_segment_t seg[VSCSIIF_SG_TABLESIZE]; + uint32_t reserved[3]; +}; Could you add a comment saying what the size should be. Just in case somebody extends this in the future - they will know the limits. Okay. +typedef struct vscsiif_request vscsiif_request_t; Ditto. + +#define VSCSIIF_SG_LIST_SIZE ((sizeof(vscsiif_request_t) - 4) / \ + sizeof(vscsiif_segment_t)) That -4 ought to have a comment. In case
Re: [Xen-devel] [PATCH 1/4] Add XEN pvSCSI protocol description
On 06/30/2014 01:02 PM, Jan Beulich wrote: On 27.06.14 at 19:11, konrad.w...@oracle.com wrote: On Fri, Jun 27, 2014 at 04:34:33PM +0200, jgr...@suse.com wrote: +/* + * Maximum scatter/gather segments per request. + * + * Considering balance between allocating at least 16 vscsiif_request + * structures on one page (4096 bytes) and the number of scatter/gather + * elements needed, we decided to use 26 as a magic number. + */ +#define VSCSIIF_SG_TABLESIZE 26 + +/* + * based on Linux kernel 2.6.18 This being a bit more .. new, - do these sizes make sense anymore? Should they be extended a bit? Or have support for using the old ones (as default) and then negotiate new sizes with the kernel? (If of course there is a difference?) As Jürgen already said (and as you should have noticed yourself) - this is an interface definition that we can't just change. Negotiation of larger counts is an option, which is what VSCSIIF_ACT_SCSI_SG_PRESET is intended for (the implementation of which isn't part of this patchset afaics, but could be made available on top of it). I don't think this is the proper way to handle larger SG lists. The VSCSIIF_ACT_SCSI_SG_PRESET option would just bump the maximum length up to 31 (currently 26). Or was it thought to be incremental (multiple presets for one request)? I'd rather add a way to specify SG lists residing in an own (granted) page (or even multiple pages). This would allow 512*26 SG entries without having to change the request structure. The capability to handle this feature could be indicated via xenstore. Most if not all of your other comments are a little questionable too in this context - any parts you aren't happy about would really better be addressed towards the canonical header in the Xen tree. (I know you had other interface headers diverge too in their Linux incarnation, but personally I don't think this is an appropriate thing to do, perhaps apart for pure coding style adjustments.) Juergen -- 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: [Xen-devel] [PATCH 1/4] Add XEN pvSCSI protocol description
On 30.06.14 at 13:18, jgr...@suse.com wrote: On 06/30/2014 01:02 PM, Jan Beulich wrote: On 27.06.14 at 19:11, konrad.w...@oracle.com wrote: On Fri, Jun 27, 2014 at 04:34:33PM +0200, jgr...@suse.com wrote: +/* + * Maximum scatter/gather segments per request. + * + * Considering balance between allocating at least 16 vscsiif_request + * structures on one page (4096 bytes) and the number of scatter/gather + * elements needed, we decided to use 26 as a magic number. + */ +#define VSCSIIF_SG_TABLESIZE 26 + +/* + * based on Linux kernel 2.6.18 This being a bit more .. new, - do these sizes make sense anymore? Should they be extended a bit? Or have support for using the old ones (as default) and then negotiate new sizes with the kernel? (If of course there is a difference?) As Jürgen already said (and as you should have noticed yourself) - this is an interface definition that we can't just change. Negotiation of larger counts is an option, which is what VSCSIIF_ACT_SCSI_SG_PRESET is intended for (the implementation of which isn't part of this patchset afaics, but could be made available on top of it). I don't think this is the proper way to handle larger SG lists. The VSCSIIF_ACT_SCSI_SG_PRESET option would just bump the maximum length up to 31 (currently 26). Or was it thought to be incremental (multiple presets for one request)? Yes, that's how it's to be used. See xen-vscsi-large-requests.patch it the newer of our trees. I'd rather add a way to specify SG lists residing in an own (granted) page (or even multiple pages). This would allow 512*26 SG entries without having to change the request structure. The capability to handle this feature could be indicated via xenstore. And yes, considering the similar changes in blkif I agree that this would be the preferred method today. That patch, however, predates those blkif enhancements. Jan -- 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: [Xen-devel] [PATCH 2/4] Introduce xen-scsifront module
On 27/06/14 15:34, jgr...@suse.com wrote: +/* .resume = scsifront_resume, */ Following on from the recent discussion on migration with front/backends I thought I'd review how this new driver does it. Is there an expectation that a VM with a PV scsi device cannot be restored or migrated? David -- 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: [Xen-devel] [PATCH 2/4] Introduce xen-scsifront module
On 06/30/2014 03:35 PM, David Vrabel wrote: On 27/06/14 15:34, jgr...@suse.com wrote: +/* .resume = scsifront_resume, */ Following on from the recent discussion on migration with front/backends I thought I'd review how this new driver does it. New is an attribute I wouldn't use in this case. The driver was introduced in 2008, this is just a rework to be able to put it in the upstream kernel. Is there an expectation that a VM with a PV scsi device cannot be restored or migrated? It has been so in the past. To change this is on my todo list, but that's not top priority. Juergen -- 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: External USB3 disk fails with Invalid field in cdb
On Fri, 27 Jun 2014 21:52:35 +0200 Michael Büsch m...@bues.ch wrote: On Fri, 27 Jun 2014 15:23:42 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: Does the patch below do what you and James want? Yes, this does work. Thanks! You can add my Tested-by: Michael Büsch m...@bues.ch Does somebody pick this up for inclusion? It'd also be good, if this hit stable. -- Michael signature.asc Description: PGP signature
Re: [usb-storage] Re: External USB3 disk fails with Invalid field in cdb
On Fri, 2014-06-27 at 15:23 -0400, Alan Stern wrote: On Fri, 27 Jun 2014, Michael Büsch wrote: On Fri, 27 Jun 2014 14:42:01 -0400 (EDT) Alan Stern st...@rowland.harvard.edu wrote: Michael, can you post the lsusb -v output for this device? I see it is made by JMicron; they are notorious for buggy USB-ATA bridges. Of course. Here you go: Bus 004 Device 009: ID 152d:0567 JMicron Technology Corp. / JMicron USA Technology Corp. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x152d JMicron Technology Corp. / JMicron USA Technology Corp. idProduct 0x0567 bcdDevice1.14 iManufacturer 1 JMicron iProduct2 USB to ATA/ATAPI Bridge iSerial 3 xxx bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 121 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 4 USB Mass Storage bmAttributes 0xc0 Self Powered MaxPower2mA ... MaxPower=2mA is a nice guess for a hard disk. ;) That refers to the amount of power the device draws from the USB bus. Since the disk drive is self-powered, it doesn't use much bus power. Does the patch below do what you and James want? Yes, that's the usual annoying additions to our blacklist. You can add my acked-by and could you cc stable? Thanks, 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: [usb-storage] Re: External USB3 disk fails with Invalid field in cdb
On Mon, 30 Jun 2014, James Bottomley wrote: Does the patch below do what you and James want? Yes, that's the usual annoying additions to our blacklist. You can add my acked-by and could you cc stable? Will do. 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
[PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
Some buggy JMicron USB-ATA bridges don't know how to translate the FUA bit in READs or WRITEs. This patch adds an entry in unusual_devs.h and a blacklist flag to tell the sd driver not to use FUA. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Michael Büsch m...@bues.ch Tested-by: Michael Büsch m...@bues.ch Acked-by: James Bottomley james.bottom...@hansenpartnership.com CC: Matthew Dharm mdharm-...@one-eyed-alien.net CC: sta...@vger.kernel.org --- [as1751] drivers/scsi/sd.c |5 - drivers/usb/storage/scsiglue.c |4 drivers/usb/storage/unusual_devs.h |7 +++ include/linux/usb_usual.h |4 +++- include/scsi/scsi_device.h |1 + 5 files changed, 19 insertions(+), 2 deletions(-) Index: usb-3.16/include/linux/usb_usual.h === --- usb-3.16.orig/include/linux/usb_usual.h +++ usb-3.16/include/linux/usb_usual.h @@ -70,7 +70,9 @@ US_FLAG(NEEDS_CAP16,0x0040) \ /* cannot handle READ_CAPACITY_10 */\ US_FLAG(IGNORE_UAS, 0x0080) \ - /* Device advertises UAS but it is broken */ + /* Device advertises UAS but it is broken */\ + US_FLAG(BROKEN_FUA, 0x0100) \ + /* Cannot handle FUA in WRITE or READ CDBs */ \ #define US_FLAG(name, value) US_FL_##name = value , enum { US_DO_ALL_FLAGS }; Index: usb-3.16/include/scsi/scsi_device.h === --- usb-3.16.orig/include/scsi/scsi_device.h +++ usb-3.16/include/scsi/scsi_device.h @@ -173,6 +173,7 @@ struct scsi_device { unsigned is_visible:1; /* is the device visible in sysfs */ unsigned wce_default_on:1; /* Cache is ON by default */ unsigned no_dif:1; /* T10 PI (DIF) should be disabled */ + unsigned broken_fua:1; /* Don't set FUA bit */ atomic_t disk_events_disable_depth; /* disable depth for disk events */ Index: usb-3.16/drivers/usb/storage/unusual_devs.h === --- usb-3.16.orig/drivers/usb/storage/unusual_devs.h +++ usb-3.16/drivers/usb/storage/unusual_devs.h @@ -1936,6 +1936,13 @@ UNUSUAL_DEV( 0x14cd, 0x6600, 0x0201, 0x USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_IGNORE_RESIDUE ), +/* Reported by Michael Büsch m...@bues.ch */ +UNUSUAL_DEV( 0x152d, 0x0567, 0x0114, 0x0114, + JMicron, + USB to ATA/ATAPI Bridge, + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_BROKEN_FUA ), + /* Reported by Alexandre Oliva ol...@lsd.ic.unicamp.br * JMicron responds to USN and several other SCSI ioctls with a * residue that causes subsequent I/O requests to fail. */ Index: usb-3.16/drivers/usb/storage/scsiglue.c === --- usb-3.16.orig/drivers/usb/storage/scsiglue.c +++ usb-3.16/drivers/usb/storage/scsiglue.c @@ -256,6 +256,10 @@ static int slave_configure(struct scsi_d if (us-fflags US_FL_WRITE_CACHE) sdev-wce_default_on = 1; + /* A few buggy USB-ATA bridges don't understand FUA */ + if (us-fflags US_FL_BROKEN_FUA) + sdev-broken_fua = 1; + } else { /* Non-disk-type devices don't need to blacklist any pages Index: usb-3.16/drivers/scsi/sd.c === --- usb-3.16.orig/drivers/scsi/sd.c +++ usb-3.16/drivers/scsi/sd.c @@ -2441,7 +2441,10 @@ sd_read_cache_type(struct scsi_disk *sdk } sdkp-DPOFUA = (data.device_specific 0x10) != 0; - if (sdkp-DPOFUA !sdkp-device-use_10_for_rw) { + if (sdp-broken_fua) { + sd_first_printk(KERN_NOTICE, sdkp, Disabling FUA\n); + sdkp-DPOFUA = 0; + } else if (sdkp-DPOFUA !sdkp-device-use_10_for_rw) { sd_first_printk(KERN_NOTICE, sdkp, Uses READ/WRITE(6), disabling FUA\n); sdkp-DPOFUA = 0; -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
On Mon, 30 Jun 2014, Alan Stern wrote: Some buggy JMicron USB-ATA bridges don't know how to translate the FUA bit in READs or WRITEs. This patch adds an entry in unusual_devs.h and a blacklist flag to tell the sd driver not to use FUA. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Michael Büsch m...@bues.ch Tested-by: Michael Büsch m...@bues.ch Acked-by: James Bottomley james.bottom...@hansenpartnership.com CC: Matthew Dharm mdharm-...@one-eyed-alien.net CC: sta...@vger.kernel.org Greg: I forgot to add your name to the recipient list on this patch. Since James has already Ack'ed it, would you like to take it for your USB tree? 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: scsi-mq V2
On 06/25/2014 10:50 PM, Jens Axboe wrote: On 2014-06-25 10:51, Christoph Hellwig wrote: This is the second post of the scsi-mq series. At this point the code is ready for merging and use by developers and early adopters. The core blk-mq code isn't that suitable for slow devices yet, mostly due to the lack of an I/O scheduler, but Jens is working on it. Similarly there is no dm-multipath support for drivers using blk-mq yet, but I'm working on it. It should also be noted that the code doesn't actually support multiple hardware queues or fine grained tuning of the blk-mq parameters yet. All these could be added fairly easily as soon as low-level drivers want to make use of them. The amount of chances to the existing code are fairly small, and mostly speedups or cleanups that also apply to the old path as well. Because of this I also haven't bothered to put it under a config option, just like the blk-mq core. The usage of blk-mq dramatically decreases CPU usage under all workloads going down from 100% CPU usage that the old setup can hit easily to usually less than 20% for maxing out storage subsystems with 512byte reads and writes, and it allows to easily archive millions of IOPS. Bart and Robert have helped with some very detailed measurements that they might be able to send in reply to this, although these usually involve significantly reworked low level drivers to avoid other bottle necks. One major objection to previous iterations of this code was the simple replacement of the host_lock with atomic counters for the host and busy counters. The host_lock avoidance on it's own already improves performance, and with the patch to avoid maintaining the per-target busy counter unless needed we now replace a lock round trip on the host_lock with just a single atomic increment in the submission path, and a single atomic decrement in completion path, which should provide benefits even for the oddest RISC architecture. Longer term I'd still love to get rid of these entirely and use the counters in blk-mq, but due to the difference in how they are maintained this doesn't seem feasible as long as we still need to support the legacy request code path. Changes from V1: - rebased on top of the core-for-3.17 branch, most notable the scsi logging changes - fixed handling of cmd_list to prevent crashes for some heavy workloads - fixed incorrect handling of !target-can_queue - avoid scheduling a workqueue on I/O completions when no queues are congested In addition to the patches in this thread there also is a git available at: git://git.infradead.org/users/hch/scsi.git scsi-mq.2 You can add my acked/reviewed-by to the series. Ran stress testing from Friday to now, 65h of beating up on it and no problems observed. 47TB read and 20TB written for a total of 17.7 billion of IOs issued and completed. Latencies look good. I officially declare this code for bug free. Bug-free-by: Jens Axboe ax...@fb.com Now lets get this queued up for inclusion, pretty please. -- Jens Axboe -- 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-mq V2
On Mon, Jun 30, 2014 at 09:20:51AM -0600, Jens Axboe wrote: Ran stress testing from Friday to now, 65h of beating up on it and no problems observed. 47TB read and 20TB written for a total of 17.7 billion of IOs issued and completed. Latencies look good. I officially declare this code for bug free. Bug-free-by: Jens Axboe ax...@fb.com Now lets get this queued up for inclusion, pretty please. I'm still looking for one (or better two) persons familar with the SCSI and/or block code to go over it and do a real detailed review. -- 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-mq V2
Christoph == Christoph Hellwig h...@infradead.org writes: Christoph I'm still looking for one (or better two) persons familar Christoph with the SCSI and/or block code to go over it and do a real Christoph detailed review. I'm on vacation for a couple of days. Will review Wednesday. -- 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: [PATCH] usb-storage/SCSI: Add broken_fua blacklist flag
On Mon, Jun 30, 2014 at 11:16:51AM -0400, Alan Stern wrote: On Mon, 30 Jun 2014, Alan Stern wrote: Some buggy JMicron USB-ATA bridges don't know how to translate the FUA bit in READs or WRITEs. This patch adds an entry in unusual_devs.h and a blacklist flag to tell the sd driver not to use FUA. Signed-off-by: Alan Stern st...@rowland.harvard.edu Reported-by: Michael Büsch m...@bues.ch Tested-by: Michael Büsch m...@bues.ch Acked-by: James Bottomley james.bottom...@hansenpartnership.com CC: Matthew Dharm mdharm-...@one-eyed-alien.net CC: sta...@vger.kernel.org Greg: I forgot to add your name to the recipient list on this patch. Since James has already Ack'ed it, would you like to take it for your USB tree? Sure, I can take it, thanks. greg k-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
[PATCH 1/1] [SCSI] bfa: use ARRAY_SIZE instead of sizeof/sizeof[0]
Use macro definition Cc: Anil Gurumurthy anil.gurumur...@qlogic.com Cc: Sudarsana Kalluru sudarsana.kall...@qlogic.com Cc: James E.J. Bottomley jbottom...@parallels.com Cc: linux-scsi@vger.kernel.org Signed-off-by: Fabian Frederick f...@skynet.be --- drivers/scsi/bfa/bfa_fcs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/bfa/bfa_fcs.c b/drivers/scsi/bfa/bfa_fcs.c index a3ab5cc..0f19455 100644 --- a/drivers/scsi/bfa/bfa_fcs.c +++ b/drivers/scsi/bfa/bfa_fcs.c @@ -81,7 +81,7 @@ bfa_fcs_attach(struct bfa_fcs_s *fcs, struct bfa_s *bfa, struct bfad_s *bfad, bfa-fcs = BFA_TRUE; fcbuild_init(); - for (i = 0; i sizeof(fcs_modules) / sizeof(fcs_modules[0]); i++) { + for (i = 0; i ARRAY_SIZE(fcs_modules); i++) { mod = fcs_modules[i]; if (mod-attach) mod-attach(fcs); @@ -97,7 +97,7 @@ bfa_fcs_init(struct bfa_fcs_s *fcs) int i; struct bfa_fcs_mod_s *mod; - for (i = 0; i sizeof(fcs_modules) / sizeof(fcs_modules[0]); i++) { + for (i = 0; i ARRAY_SIZE(fcs_modules); i++) { mod = fcs_modules[i]; if (mod-modinit) mod-modinit(fcs); @@ -184,7 +184,7 @@ bfa_fcs_exit(struct bfa_fcs_s *fcs) bfa_wc_init(fcs-wc, bfa_fcs_exit_comp, fcs); - nmods = sizeof(fcs_modules) / sizeof(fcs_modules[0]); + nmods = ARRAY_SIZE(fcs_modules); for (i = 0; i nmods; i++) { -- 1.8.4.5 -- 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 eats error from flush failure during hot plug
Our qual rig is having some woes. I'll let you know how the test works out after I finish resuscitating it. On Mon, Jun 30, 2014 at 11:44 AM, Steven Haber ste...@qumulo.com wrote: Our qual rig is having some woes. I'll let you know how the test works out after I finish resuscitating it. On Fri, Jun 27, 2014 at 12:55 AM, Christoph Hellwig h...@infradead.org wrote: On Thu, Jun 26, 2014 at 02:52:14PM -0400, James Bottomley wrote: + } else if (req-cmd_flags REQ_FLUSH) { + /* +* Flush request don't transfer data, so we can't try +* to complete the good bytes first before checking +* the error. +*/ + if (result !sense_deferred) + error = __scsi_error_from_host_byte(cmd, result); Well, no, that's wrong. To catch all of the corner cases that slip through here, the check is for a zero length command with and error otherwise we get the same problem for failed discards. It's correct for the current code base where FLUSH is the only zero length !BLOCK_PC case. Your version is fine too, except for the incorrect comment: + } else if (blk_rq_bytes(req) == 0 result !sense_deferred) { + /* + * Certain non BLOCK_PC requests are commands that don't + * actually transfer anything (FLUSH, DISCARD), so cannot use + * good_bytes == 0 as the signal for an error. This sets the + * error explicitly for the problem case + */ There aren't BLOCK_PC commands, otherwise they wouldn't end up here. Note that REQ_DISCARD has been rewritten into either an WRITE_SAME or UNMAP at this point, which always transfer data. -- 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
[PATCHv3 7/8] target: Remove core_tpg_release_virtual_lun0 function
Simple and just called from one place. Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_tpg.c | 9 + 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index a63a3d7..bc0299a 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -591,13 +591,6 @@ static int core_tpg_setup_virtual_lun0(struct se_portal_group *se_tpg) return 0; } -static void core_tpg_release_virtual_lun0(struct se_portal_group *se_tpg) -{ - struct se_lun *lun = se_tpg-tpg_virt_lun0; - - core_tpg_remove_lun(se_tpg, lun); -} - int core_tpg_register( struct target_core_fabric_ops *tfo, struct se_wwn *se_wwn, @@ -673,7 +666,7 @@ int core_tpg_deregister(struct se_portal_group *se_tpg) spin_unlock_irq(se_tpg-acl_node_lock); if (se_tpg-se_tpg_type == TRANSPORT_TPG_TYPE_NORMAL) - core_tpg_release_virtual_lun0(se_tpg); + core_tpg_remove_lun(se_tpg, se_tpg-tpg_virt_lun0); se_tpg-se_tpg_fabric_ptr = NULL; -- 1.9.3 -- 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
[PATCHv3 6/8] target: Allocate se_luns only when used
Instead of allocating the tpg_lun_list and each member of the list when the tpg is created, make the list part of the parent struct, and use an element being non-NULL to indicate an active LUN. This will save memory if less than all the se_luns are actually configured. Now that things are actually getting freed, split out core_tpg_free_lun from core_tpg_remove_lun, because we don't want to free() the statically- allocated virtual_lun0. Remove array_free and array_zalloc. Remove core_get_lun_from_tpg. Change core_dev_add_lun to take a se_lun and return int Do not allocate tpg_lun_list, put it directly in se_portal_group. Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/sbp/sbp_target.c | 6 +- drivers/target/target_core_device.c | 43 ++-- drivers/target/target_core_fabric_configfs.c | 21 +++--- drivers/target/target_core_internal.h| 4 +- drivers/target/target_core_tpg.c | 101 +-- include/target/target_core_base.h| 10 +-- 6 files changed, 54 insertions(+), 131 deletions(-) diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c index e7e9372..62b9d47 100644 --- a/drivers/target/sbp/sbp_target.c +++ b/drivers/target/sbp/sbp_target.c @@ -187,7 +187,7 @@ static struct se_lun *sbp_get_lun_from_tpg(struct sbp_tpg *tpg, int lun) spin_lock(se_tpg-tpg_lun_lock); se_lun = se_tpg-tpg_lun_list[lun]; - if (se_lun-lun_status != TRANSPORT_LUN_STATUS_ACTIVE) + if (!se_lun) se_lun = ERR_PTR(-ENODEV); spin_unlock(se_tpg-tpg_lun_lock); @@ -1947,7 +1947,7 @@ static int sbp_count_se_tpg_luns(struct se_portal_group *tpg) for (i = 0; i TRANSPORT_MAX_LUNS_PER_TPG; i++) { struct se_lun *se_lun = tpg-tpg_lun_list[i]; - if (se_lun-lun_status == TRANSPORT_LUN_STATUS_FREE) + if (!se_lun) continue; count++; @@ -2027,7 +2027,7 @@ static int sbp_update_unit_directory(struct sbp_tport *tport) struct se_device *dev; int type; - if (se_lun-lun_status == TRANSPORT_LUN_STATUS_FREE) + if (!se_lun) continue; spin_unlock(tport-tpg-se_tpg.tpg_lun_lock); diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index bd467a4..4b8149b 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1217,22 +1217,17 @@ int se_dev_set_block_size(struct se_device *dev, u32 block_size) return 0; } -struct se_lun *core_dev_add_lun( +int core_dev_add_lun( struct se_portal_group *tpg, struct se_device *dev, - u32 unpacked_lun) + struct se_lun *lun) { - struct se_lun *lun; int rc; - lun = core_tpg_alloc_lun(tpg, unpacked_lun); - if (IS_ERR(lun)) - return lun; - rc = core_tpg_add_lun(tpg, lun, TRANSPORT_LUNFLAGS_READ_WRITE, dev); if (rc 0) - return ERR_PTR(rc); + return rc; pr_debug(%s_TPG[%u]_LUN[%u] - Activated %s Logical Unit from CORE HBA: %u\n, tpg-se_tpg_tfo-get_fabric_name(), @@ -1257,7 +1252,7 @@ struct se_lun *core_dev_add_lun( spin_unlock_irq(tpg-acl_node_lock); } - return lun; + return rc; } /* core_dev_del_lun(): @@ -1274,35 +1269,7 @@ void core_dev_del_lun( tpg-se_tpg_tfo-get_fabric_name()); core_tpg_remove_lun(tpg, lun); -} - -struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun) -{ - struct se_lun *lun; - - spin_lock(tpg-tpg_lun_lock); - if (unpacked_lun (TRANSPORT_MAX_LUNS_PER_TPG-1)) { - pr_err(%s LUN: %u exceeds TRANSPORT_MAX_LUNS - _PER_TPG-1: %u for Target Portal Group: %hu\n, - tpg-se_tpg_tfo-get_fabric_name(), unpacked_lun, - TRANSPORT_MAX_LUNS_PER_TPG-1, - tpg-se_tpg_tfo-tpg_get_tag(tpg)); - spin_unlock(tpg-tpg_lun_lock); - return NULL; - } - lun = tpg-tpg_lun_list[unpacked_lun]; - - if (lun-lun_status != TRANSPORT_LUN_STATUS_FREE) { - pr_err(%s Logical Unit Number: %u is not free on -Target Portal Group: %hu, ignoring request.\n, - tpg-se_tpg_tfo-get_fabric_name(), unpacked_lun, - tpg-se_tpg_tfo-tpg_get_tag(tpg)); - spin_unlock(tpg-tpg_lun_lock); - return NULL; - } - spin_unlock(tpg-tpg_lun_lock); - - return lun; + core_tpg_free_lun(tpg, lun); } struct se_lun_acl *core_dev_init_initiator_node_lun_acl( diff --git
[PATCHv3 1/8] target: Add locking to some accesses to nacl.device_list
Make sure all accesses of deve elements in device_list are protected. This will become critically important once device_list entries are potentially NULL. Reported-by: Christoph Hellwig h...@lst.de Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_device.c | 7 ++--- drivers/target/target_core_fabric_configfs.c | 10 +-- drivers/target/target_core_pr.c | 40 +--- drivers/target/target_core_ua.c | 3 +++ 4 files changed, 46 insertions(+), 14 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 11d26fe..10130ea 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -361,11 +361,12 @@ int core_enable_device_list_for_node( deve-creation_time = get_jiffies_64(); deve-attach_count++; - spin_unlock_irq(nacl-device_list_lock); - spin_lock_bh(port-sep_alua_lock); + spin_lock(port-sep_alua_lock); list_add_tail(deve-alua_port_list, port-sep_alua_list); - spin_unlock_bh(port-sep_alua_lock); + spin_unlock(port-sep_alua_lock); + + spin_unlock_irq(nacl-device_list_lock); return 0; } diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 7de9f04..804e7f0 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -141,13 +141,19 @@ static int target_fabric_mappedlun_unlink( struct se_lun_acl *lacl = container_of(to_config_group(lun_acl_ci), struct se_lun_acl, se_lun_group); struct se_node_acl *nacl = lacl-se_lun_nacl; - struct se_dev_entry *deve = nacl-device_list[lacl-mapped_lun]; + struct se_dev_entry *deve; struct se_portal_group *se_tpg; + + spin_lock_irq(nacl-device_list_lock); + deve = nacl-device_list[lacl-mapped_lun]; /* * Determine if the underlying MappedLUN has already been released.. */ - if (!deve-se_lun) + if (!deve-se_lun) { + spin_unlock_irq(nacl-device_list_lock); return 0; + } + spin_unlock_irq(nacl-device_list_lock); lun = container_of(to_config_group(lun_ci), struct se_lun, lun_group); se_tpg = lun-lun_sep-sep_tpg; diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index df35786..c647ca9 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -311,8 +311,13 @@ static int core_scsi3_pr_seq_non_holder( int we = 0; /* Write Exclusive */ int legacy = 0; /* Act like a legacy device and return * RESERVATION CONFLICT on some CDBs */ + int def_pr_registered; + spin_lock_irq(se_sess-se_node_acl-device_list_lock); se_deve = se_sess-se_node_acl-device_list[cmd-orig_fe_lun]; + def_pr_registered = se_deve-def_pr_registered; + spin_unlock_irq(se_sess-se_node_acl-device_list_lock); + /* * Determine if the registration should be ignored due to * non-matching ISIDs in target_scsi3_pr_reservation_check(). @@ -329,7 +334,7 @@ static int core_scsi3_pr_seq_non_holder( * Some commands are only allowed for the persistent reservation * holder. */ - if ((se_deve-def_pr_registered) !(ignore_reg)) + if ((def_pr_registered) !(ignore_reg)) registered_nexus = 1; break; case PR_TYPE_WRITE_EXCLUSIVE_REGONLY: @@ -339,7 +344,7 @@ static int core_scsi3_pr_seq_non_holder( * Some commands are only allowed for registered I_T Nexuses. */ reg_only = 1; - if ((se_deve-def_pr_registered) !(ignore_reg)) + if ((def_pr_registered) !(ignore_reg)) registered_nexus = 1; break; case PR_TYPE_WRITE_EXCLUSIVE_ALLREG: @@ -349,7 +354,7 @@ static int core_scsi3_pr_seq_non_holder( * Each registered I_T Nexus is a reservation holder. */ all_reg = 1; - if ((se_deve-def_pr_registered) !(ignore_reg)) + if ((def_pr_registered) !(ignore_reg)) registered_nexus = 1; break; default: @@ -947,13 +952,21 @@ int core_scsi3_check_aptpl_registration( struct se_lun_acl *lun_acl) { struct se_node_acl *nacl = lun_acl-se_lun_nacl; - struct se_dev_entry *deve = nacl-device_list[lun_acl-mapped_lun]; + struct se_dev_entry *deve; + int ret; if (dev-dev_reservation_flags DRF_SPC2_RESERVATIONS) return 0; - return __core_scsi3_check_aptpl_registration(dev, tpg, lun, - lun-unpacked_lun, nacl, deve); +
[PATCHv3 8/8] target: Refactor core_enable_device_list_for_node
Create helper functions to alloc a deve and to transition it from demo mode to explicit. Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_device.c | 121 ++-- 1 file changed, 74 insertions(+), 47 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 4b8149b..2c589a5 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -288,6 +288,73 @@ void core_update_device_list_access( spin_unlock_irq(nacl-device_list_lock); } +static struct se_dev_entry *core_alloc_se_dev_entry( + struct se_lun *lun, + struct se_lun_acl *lun_acl, + u32 mapped_lun, + u32 lun_access, + struct se_node_acl *nacl) +{ + struct se_dev_entry *deve; + + /* Holding locks, can't sleep */ + deve = kzalloc(sizeof(*deve), GFP_ATOMIC); + if (!deve) + return ERR_PTR(-ENOMEM); + + atomic_set(deve-ua_count, 0); + atomic_set(deve-pr_ref_count, 0); + spin_lock_init(deve-ua_lock); + INIT_LIST_HEAD(deve-alua_port_list); + INIT_LIST_HEAD(deve-ua_list); + deve-se_lun = lun; + deve-se_lun_acl = lun_acl; + deve-mapped_lun = mapped_lun; + + if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; + else + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; + + deve-creation_time = get_jiffies_64(); + + nacl-device_list[mapped_lun] = deve; + + return deve; +} + +static int core_transition_deve_to_explicit( + struct se_lun *lun, + struct se_lun_acl *lun_acl, + struct se_dev_entry *deve, + u32 lun_access) +{ + if (deve-se_lun_acl != NULL) { + pr_err(struct se_dev_entry-se_lun_acl + already set for demo mode - explicit + LUN ACL transition\n); + return -EINVAL; + } + if (deve-se_lun != lun) { + pr_err(struct se_dev_entry-se_lun does + match passed struct se_lun for demo mode + - explicit LUN ACL transition\n); + return -EINVAL; + } + + deve-se_lun_acl = lun_acl; + + if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) { + deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_ONLY; + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; + } else { + deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_WRITE; + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; + } + + return 0; +} + /* core_enable_device_list_for_node(): * */ @@ -316,62 +383,22 @@ int core_enable_device_list_for_node( * + mapped_lun that was setup in demo mode.. */ if (deve) { - if (deve-se_lun_acl != NULL) { - pr_err(struct se_dev_entry-se_lun_acl - already set for demo mode - explicit - LUN ACL transition\n); - ret = -EINVAL; - goto out; - } - if (deve-se_lun != lun) { - pr_err(struct se_dev_entry-se_lun does - match passed struct se_lun for demo mode - - explicit LUN ACL transition\n); - ret = -EINVAL; - goto out; - } - deve-se_lun_acl = lun_acl; - - if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) { - deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_ONLY; - deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; - } else { - deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_WRITE; - deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; - } - + ret = core_transition_deve_to_explicit(lun, lun_acl, deve, + lun_access); goto out; } - deve = kzalloc(sizeof(*deve), GFP_ATOMIC); - if (!deve) { - spin_unlock_irq(nacl-device_list_lock); - return -ENOMEM; + deve = core_alloc_se_dev_entry(lun, lun_acl, mapped_lun, lun_access, nacl); + if (IS_ERR(deve)) { + ret = PTR_ERR(deve); + goto out; } - nacl-device_list[mapped_lun] = deve; - - atomic_set(deve-ua_count, 0); - atomic_set(deve-pr_ref_count, 0); - spin_lock_init(deve-ua_lock); - INIT_LIST_HEAD(deve-alua_port_list); - INIT_LIST_HEAD(deve-ua_list); - deve-se_lun = lun; - deve-se_lun_acl = lun_acl; - deve-mapped_lun = mapped_lun; - - if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) - deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; - else -
[PATCHv3 0/8] target: Save memory on unused se_dev_entrys and se_luns
Hi nab, hch, and all, This patchset reduces the amount of memory for se_dev_entry and se_lun arrays by waiting to allocate array members until they are created. This patch saves up to 261KB per TPG, and up to 65KB per ACL. It also fixes a number of locking bugs around these data structures. Changes since v2, based on hch's review: * Fix braces and add precondition checking in core_enable_device_list_for_node() in patch 2 * Move busy-loop outside of spinlock in core_disable_device_list_for_node() in patch 3 * Merge patches 5 and 6 Against target-pending/for-next (3.16-rc2). Thanks -- Regards -- Andy Andy Grover (8): target: Add locking to some accesses to nacl.device_list target: Don't unlock/relock tpg_lun_lock in loop in add_node_to_devs target: Allocate se_dev_entrys in device list only when used target: core_tpg_post_dellun can return void target: Change core_dev_del_lun to take a se_lun instead of unpacked_lun target: Allocate se_luns only when used target: Remove core_tpg_release_virtual_lun0 function target: Refactor core_enable_device_list_for_node drivers/target/sbp/sbp_target.c | 6 +- drivers/target/target_core_device.c | 313 +-- drivers/target/target_core_fabric_configfs.c | 35 +-- drivers/target/target_core_internal.h| 9 +- drivers/target/target_core_pr.c | 40 +++- drivers/target/target_core_spc.c | 2 +- drivers/target/target_core_tpg.c | 189 +++- drivers/target/target_core_ua.c | 3 + include/target/target_core_base.h| 17 +- 9 files changed, 248 insertions(+), 366 deletions(-) -- 1.9.3 -- 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
[PATCHv3 5/8] target: Change core_dev_del_lun to take a se_lun instead of unpacked_lun
Remove core_tpg_pre_dellun entirely, since we don't need to get/check a pointer we already have. Nothing else can return an error, so core_dev_del_lun can return void. Rename core_tpg_post_dellun to remove_lun - a clearer name, now that pre_dellun is gone. Reviewed-by: Christoph Hellwig h...@lst.de Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_device.c | 18 -- drivers/target/target_core_fabric_configfs.c | 2 +- drivers/target/target_core_internal.h| 5 ++-- drivers/target/target_core_tpg.c | 36 +++- 4 files changed, 11 insertions(+), 50 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 0bac890..bd467a4 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -1264,24 +1264,16 @@ struct se_lun *core_dev_add_lun( * * */ -int core_dev_del_lun( +void core_dev_del_lun( struct se_portal_group *tpg, - u32 unpacked_lun) + struct se_lun *lun) { - struct se_lun *lun; - - lun = core_tpg_pre_dellun(tpg, unpacked_lun); - if (IS_ERR(lun)) - return PTR_ERR(lun); - - core_tpg_post_dellun(tpg, lun); - - pr_debug(%s_TPG[%u]_LUN[%u] - Deactivated %s Logical Unit from + pr_debug(%s_TPG[%u]_LUN[%u] - Deactivating %s Logical Unit from device object\n, tpg-se_tpg_tfo-get_fabric_name(), - tpg-se_tpg_tfo-tpg_get_tag(tpg), unpacked_lun, + tpg-se_tpg_tfo-tpg_get_tag(tpg), lun-unpacked_lun, tpg-se_tpg_tfo-get_fabric_name()); - return 0; + core_tpg_remove_lun(tpg, lun); } struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun) diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c index 0955c948..811b2f4 100644 --- a/drivers/target/target_core_fabric_configfs.c +++ b/drivers/target/target_core_fabric_configfs.c @@ -827,7 +827,7 @@ static int target_fabric_port_unlink( tf-tf_ops.fabric_pre_unlink(se_tpg, lun); } - core_dev_del_lun(se_tpg, lun-unpacked_lun); + core_dev_del_lun(se_tpg, lun); return 0; } diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h index 463fddc..42ef4ab 100644 --- a/drivers/target/target_core_internal.h +++ b/drivers/target/target_core_internal.h @@ -46,7 +46,7 @@ int se_dev_set_fabric_max_sectors(struct se_device *, u32); intse_dev_set_optimal_sectors(struct se_device *, u32); intse_dev_set_block_size(struct se_device *, u32); struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_device *, u32); -intcore_dev_del_lun(struct se_portal_group *, u32); +void core_dev_del_lun(struct se_portal_group *, struct se_lun *); struct se_lun *core_get_lun_from_tpg(struct se_portal_group *, u32); struct se_lun_acl *core_dev_init_initiator_node_lun_acl(struct se_portal_group *, struct se_node_acl *, u32, int *); @@ -82,8 +82,7 @@ void core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *); struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32); intcore_tpg_add_lun(struct se_portal_group *, struct se_lun *, u32, struct se_device *); -struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32 unpacked_lun); -void core_tpg_post_dellun(struct se_portal_group *, struct se_lun *); +void core_tpg_remove_lun(struct se_portal_group *, struct se_lun *); /* target_core_transport.c */ extern struct kmem_cache *se_tmr_req_cache; diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c index 321268d..5d2c774 100644 --- a/drivers/target/target_core_tpg.c +++ b/drivers/target/target_core_tpg.c @@ -298,7 +298,7 @@ void core_tpg_clear_object_luns(struct se_portal_group *tpg) continue; spin_unlock(tpg-tpg_lun_lock); - core_dev_del_lun(tpg, lun-unpacked_lun); + core_dev_del_lun(tpg, lun); spin_lock(tpg-tpg_lun_lock); } spin_unlock(tpg-tpg_lun_lock); @@ -625,7 +625,7 @@ static void core_tpg_release_virtual_lun0(struct se_portal_group *se_tpg) { struct se_lun *lun = se_tpg-tpg_virt_lun0; - core_tpg_post_dellun(se_tpg, lun); + core_tpg_remove_lun(se_tpg, lun); } int core_tpg_register( @@ -795,37 +795,7 @@ int core_tpg_add_lun( return 0; } -struct se_lun *core_tpg_pre_dellun( - struct se_portal_group *tpg, - u32 unpacked_lun) -{ - struct se_lun *lun; - - if (unpacked_lun (TRANSPORT_MAX_LUNS_PER_TPG-1)) { - pr_err(%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER_TPG - -1: %u for Target Portal Group: %u\n, - tpg-se_tpg_tfo-get_fabric_name(), unpacked_lun, - TRANSPORT_MAX_LUNS_PER_TPG-1, -
[PATCHv3 3/8] target: Allocate se_dev_entrys in device list only when used
Remove TRANSPORT_LUNFLAGS_INITIATOR_ACCESS and just look at if a non-NULL value is in the node_acl-device_list for the given lun. Since usually nowhere near all se_dev_entrys are used, this nets a sizable reduction in memory use. In core_disable_device_list_for_node, move all calls to functions referencing deve inside the spinlock, since it's not safe to access deve outside it. Skip reinitializing, since the deve is actually being freed. Do not allocate device_list array separately, but include it in se_node_acl. Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_device.c | 93 drivers/target/target_core_fabric_configfs.c | 4 +- drivers/target/target_core_spc.c | 2 +- drivers/target/target_core_tpg.c | 41 +--- include/target/target_core_base.h| 7 +-- 5 files changed, 59 insertions(+), 88 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 5d4a8b3..0bac890 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -67,7 +67,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun) spin_lock_irqsave(se_sess-se_node_acl-device_list_lock, flags); se_cmd-se_deve = se_sess-se_node_acl-device_list[unpacked_lun]; - if (se_cmd-se_deve-lun_flags TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) { + if (se_cmd-se_deve) { struct se_dev_entry *deve = se_cmd-se_deve; deve-total_cmds++; @@ -143,7 +143,6 @@ EXPORT_SYMBOL(transport_lookup_cmd_lun); int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun) { - struct se_dev_entry *deve; struct se_lun *se_lun = NULL; struct se_session *se_sess = se_cmd-se_sess; struct se_tmr_req *se_tmr = se_cmd-se_tmr_req; @@ -154,9 +153,9 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun) spin_lock_irqsave(se_sess-se_node_acl-device_list_lock, flags); se_cmd-se_deve = se_sess-se_node_acl-device_list[unpacked_lun]; - deve = se_cmd-se_deve; + if (se_cmd-se_deve) { + struct se_dev_entry *deve = se_cmd-se_deve; - if (deve-lun_flags TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) { se_tmr-tmr_lun = deve-se_lun; se_cmd-se_lun = deve-se_lun; se_lun = deve-se_lun; @@ -204,7 +203,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi( for (i = 0; i TRANSPORT_MAX_LUNS_PER_TPG; i++) { deve = nacl-device_list[i]; - if (!(deve-lun_flags TRANSPORT_LUNFLAGS_INITIATOR_ACCESS)) + if (!deve) continue; lun = deve-se_lun; @@ -243,14 +242,11 @@ int core_free_device_list_for_node( struct se_lun *lun; u32 i; - if (!nacl-device_list) - return 0; - spin_lock_irq(nacl-device_list_lock); for (i = 0; i TRANSPORT_MAX_LUNS_PER_TPG; i++) { deve = nacl-device_list[i]; - if (!(deve-lun_flags TRANSPORT_LUNFLAGS_INITIATOR_ACCESS)) + if (!deve) continue; if (!deve-se_lun) { @@ -268,9 +264,6 @@ int core_free_device_list_for_node( } spin_unlock_irq(nacl-device_list_lock); - array_free(nacl-device_list, TRANSPORT_MAX_LUNS_PER_TPG); - nacl-device_list = NULL; - return 0; } @@ -283,12 +276,14 @@ void core_update_device_list_access( spin_lock_irq(nacl-device_list_lock); deve = nacl-device_list[mapped_lun]; - if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) { - deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_ONLY; - deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; - } else { - deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_WRITE; - deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; + if (deve) { + if (lun_access TRANSPORT_LUNFLAGS_READ_WRITE) { + deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_ONLY; + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE; + } else { + deve-lun_flags = ~TRANSPORT_LUNFLAGS_READ_WRITE; + deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; + } } spin_unlock_irq(nacl-device_list_lock); } @@ -320,7 +315,7 @@ int core_enable_device_list_for_node( * transition. This transition must be for the same struct se_lun * + mapped_lun that was setup in demo mode.. */ - if (deve-lun_flags TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) { + if (deve) { if (deve-se_lun_acl != NULL) { pr_err(struct se_dev_entry-se_lun_acl already set for demo mode - explicit @@ -348,18 +343,27 @@ int
[PATCHv3 2/8] target: Don't unlock/relock tpg_lun_lock in loop in add_node_to_devs
It's not safe to unlock/relock in core_tpg_add_node_to_devs. Remove this. As a consequence, core_enable_device_list_for_node needs to be called with a lock held irqs off. Add a comment mentioning this. Change spin_lock_irqs to spin_locks. Change error handling to goto-style. Also change the other place enable_device_list_for_node is called, add_initiator_node_lun_acl. Hold tpg_lun_lock across all uses of lun. Change error handling to release lock from error paths. Remove core_dev_get_lun. Signed-off-by: Andy Grover agro...@redhat.com --- drivers/target/target_core_device.c | 81 ++--- drivers/target/target_core_tpg.c| 3 -- 2 files changed, 31 insertions(+), 53 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 10130ea..5d4a8b3 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -295,7 +295,6 @@ void core_update_device_list_access( /* core_enable_device_list_for_node(): * - * */ int core_enable_device_list_for_node( struct se_lun *lun, @@ -307,8 +306,12 @@ int core_enable_device_list_for_node( { struct se_port *port = lun-lun_sep; struct se_dev_entry *deve; + int ret = 0; - spin_lock_irq(nacl-device_list_lock); + assert_spin_locked(tpg-tpg_lun_lock); + WARN_ON_ONCE(!irqs_disabled()); + + spin_lock(nacl-device_list_lock); deve = nacl-device_list[mapped_lun]; @@ -322,15 +325,15 @@ int core_enable_device_list_for_node( pr_err(struct se_dev_entry-se_lun_acl already set for demo mode - explicit LUN ACL transition\n); - spin_unlock_irq(nacl-device_list_lock); - return -EINVAL; + ret = -EINVAL; + goto out; } if (deve-se_lun != lun) { pr_err(struct se_dev_entry-se_lun does match passed struct se_lun for demo mode - explicit LUN ACL transition\n); - spin_unlock_irq(nacl-device_list_lock); - return -EINVAL; + ret = -EINVAL; + goto out; } deve-se_lun_acl = lun_acl; @@ -342,8 +345,7 @@ int core_enable_device_list_for_node( deve-lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY; } - spin_unlock_irq(nacl-device_list_lock); - return 0; + goto out; } deve-se_lun = lun; @@ -366,9 +368,10 @@ int core_enable_device_list_for_node( list_add_tail(deve-alua_port_list, port-sep_alua_list); spin_unlock(port-sep_alua_lock); - spin_unlock_irq(nacl-device_list_lock); +out: + spin_unlock(nacl-device_list_lock); - return 0; + return ret; } /* core_disable_device_list_for_node(): @@ -1299,39 +1302,6 @@ struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_l return lun; } -/* core_dev_get_lun(): - * - * - */ -static struct se_lun *core_dev_get_lun(struct se_portal_group *tpg, u32 unpacked_lun) -{ - struct se_lun *lun; - - spin_lock(tpg-tpg_lun_lock); - if (unpacked_lun (TRANSPORT_MAX_LUNS_PER_TPG-1)) { - pr_err(%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER - _TPG-1: %u for Target Portal Group: %hu\n, - tpg-se_tpg_tfo-get_fabric_name(), unpacked_lun, - TRANSPORT_MAX_LUNS_PER_TPG-1, - tpg-se_tpg_tfo-tpg_get_tag(tpg)); - spin_unlock(tpg-tpg_lun_lock); - return NULL; - } - lun = tpg-tpg_lun_list[unpacked_lun]; - - if (lun-lun_status != TRANSPORT_LUN_STATUS_ACTIVE) { - pr_err(%s Logical Unit Number: %u is not active on -Target Portal Group: %hu, ignoring request.\n, - tpg-se_tpg_tfo-get_fabric_name(), unpacked_lun, - tpg-se_tpg_tfo-tpg_get_tag(tpg)); - spin_unlock(tpg-tpg_lun_lock); - return NULL; - } - spin_unlock(tpg-tpg_lun_lock); - - return lun; -} - struct se_lun_acl *core_dev_init_initiator_node_lun_acl( struct se_portal_group *tpg, struct se_node_acl *nacl, @@ -1370,19 +1340,24 @@ int core_dev_add_initiator_node_lun_acl( { struct se_lun *lun; struct se_node_acl *nacl; + int ret = 0; - lun = core_dev_get_lun(tpg, unpacked_lun); + spin_lock_irq(tpg-tpg_lun_lock); + lun = tpg-tpg_lun_list[unpacked_lun]; if (!lun) { pr_err(%s Logical Unit Number: %u is not active on Target Portal Group: %hu, ignoring request.\n,
[PATCH 2/4] megaraid_sas: Removed unused variables in megasas_instance
James/linux-scsi, The following patch for megaraid_sas removes some unused variables from the megasas_instance structure. Signed-off-by: Adam Radford aradf...@gmail.com --- drivers/scsi/megaraid/megaraid_sas.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 32166c2..7d722fb 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1633,8 +1633,6 @@ struct megasas_instance { struct timer_list sriov_heartbeat_timer; char skip_heartbeat_timer_del; u8 requestorId; - u64 initiator_sas_address; - u64 ld_sas_address[64]; char PlasmaFW111; char mpio; int throttlequeuedepth; -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/4] megaraid_sas: Update for scsi for-next
(Re-sending with git-send-email on advice of Martin Petersen @ Oracle) James/linux-scsi, The following patch series for megaraid_sas brings the driver up to v06.803.02.00-rc1. 1. Fix reset_mutex leak in megasas_reset_fusion(). 2. Remove unused variables in megasas_instance. 3. Fix LD/VF affiliation parsing. 4. Version and Changelog update. Adam Radford (4): megaraid_sas: Fix reset_mutex leak megaraid_sas: Removed unused variables in megasas_instance megaraid_sas: Fix LD/VF affiliation parsing megaraid_sas: Version and Changelog update Documentation/scsi/ChangeLog.megaraid_sas | 13 + drivers/scsi/megaraid/megaraid_sas.h| 9 ++- drivers/scsi/megaraid/megaraid_sas_base.c | 90 - drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 4 files changed, 82 insertions(+), 31 deletions(-) -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] megaraid_sas: Fix reset_mutex leak
James/linux-scsi, The following patch for megaraid_sas fixes a reset_mutex leak in megasas_reset_fusion(). Signed-off-by: Adam Radford aradf...@gmail.com --- drivers/scsi/megaraid/megaraid_sas_fusion.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index 2260041..0858851 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -2355,6 +2355,7 @@ int megasas_reset_fusion(struct Scsi_Host *shost, int iotimeout) printk(KERN_WARNING megaraid_sas: Hardware critical error, returning FAILED for scsi%d.\n, instance-host-host_no); + mutex_unlock(instance-reset_mutex); return FAILED; } -- 1.7.11.7 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] megaraid_sas: Fix LD/VF affiliation parsing
James/linux-scsi, The following patch for megaraid_sas fixes the LD/VF affiliation policy parsing code to account for LD targetId's and Hidden LD's (not yet affiliated with any Virtual Functions). Signed-off-by: Adam Radford aradf...@gmail.com --- drivers/scsi/megaraid/megaraid_sas.h | 1 + drivers/scsi/megaraid/megaraid_sas_base.c | 88 ++- 2 files changed, 64 insertions(+), 25 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 7d722fb..2e2fcb2 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -1659,6 +1659,7 @@ struct MR_LD_VF_AFFILIATION { /* Plasma 1.11 FW backward compatibility structures */ #define IOV_111_OFFSET 0x7CE #define MAX_VIRTUAL_FUNCTIONS 8 +#define MR_LD_ACCESS_HIDDEN 15 struct IOV_111 { u8 maxVFsSupported; diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 112799b..8f62b30 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -1836,7 +1836,7 @@ static int megasas_get_ld_vf_affiliation(struct megasas_instance *instance, struct MR_LD_VF_MAP *newmap = NULL, *savedmap = NULL; dma_addr_t new_affiliation_h; dma_addr_t new_affiliation_111_h; - int ld, retval = 0; + int ld, i, j, retval = 0, found = 0, doscan = 0; u8 thisVf; cmd = megasas_get_cmd(instance); @@ -1944,14 +1944,6 @@ static int megasas_get_ld_vf_affiliation(struct megasas_instance *instance, if (!initial) { if (instance-PlasmaFW111) { - if (!new_affiliation_111-vdCount) { - printk(KERN_WARNING megasas: SR-IOV: Got new - LD/VF affiliation for passive path - for scsi%d.\n, - instance-host-host_no); - retval = 1; - goto out; - } thisVf = new_affiliation_111-thisVf; for (ld = 0 ; ld new_affiliation_111-vdCount; ld++) if (instance-vf_affiliation_111-map[ld].policy[thisVf] != new_affiliation_111-map[ld].policy[thisVf]) { @@ -1977,29 +1969,75 @@ static int megasas_get_ld_vf_affiliation(struct megasas_instance *instance, newmap = new_affiliation-map; savedmap = instance-vf_affiliation-map; thisVf = new_affiliation-thisVf; - for (ld = 0 ; ld new_affiliation-ldCount; ld++) { - if (savedmap-policy[thisVf] != - newmap-policy[thisVf]) { - printk(KERN_WARNING megasas: SR-IOV: - Got new LD/VF affiliation - for scsi%d.\n, - instance-host-host_no); - memcpy(instance-vf_affiliation, - new_affiliation, - new_affiliation-size); - retval = 1; + for (i = 0 ; i new_affiliation-ldCount; i++) { + found = 0; + for (j = 0; +j instance-vf_affiliation-ldCount; +j++) { + if (newmap-ref.targetId == + savedmap-ref.targetId) { + found = 1; + if (newmap-policy[thisVf] != + savedmap-policy[thisVf]) { + doscan = 1; + goto out; + } + } + savedmap = + (struct MR_LD_VF_MAP *) + ((unsigned char *)savedmap + + savedmap-size); + } + if (!found newmap-policy[thisVf] != + MR_LD_ACCESS_HIDDEN) { + doscan = 1; goto out; } - savedmap = (struct MR_LD_VF_MAP *) - ((unsigned char *)savedmap + -
[PATCH 4/4] megaraid_sas: Version and Changelog update
James/linux-scsi, The following patch for megaraid_sas updates the driver version and Documentation/scsi/ChangeLog.megaraid_sas. Signed-off-by: Adam Radford aradf...@gmail.com --- Documentation/scsi/ChangeLog.megaraid_sas | 13 + drivers/scsi/megaraid/megaraid_sas.h | 6 +++--- drivers/scsi/megaraid/megaraid_sas_base.c | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/Documentation/scsi/ChangeLog.megaraid_sas b/Documentation/scsi/ChangeLog.megaraid_sas index 91ba58e..2741efe 100644 --- a/Documentation/scsi/ChangeLog.megaraid_sas +++ b/Documentation/scsi/ChangeLog.megaraid_sas @@ -1,3 +1,16 @@ +Release Date: Thu. Jun 19, 2014 17:00:00 PST 2014 - + (emaild-id:megaraidli...@lsi.com) + Adam Radford + Kashyap Desai + Sumit Saxena + Uday Lingala +Current Version : 06.803.02.00-rc1 +Old Version : 06.803.01.00-rc1 +1. Fix reset_mutex leak in megasas_reset_fusion(). +2. Remove unused variables in megasas_instance. +3. Fix LD/VF affiliation parsing. +4. Version and Changelog update. +--- Release Date: Mon. Mar 10, 2014 17:00:00 PST 2014 - (emaild-id:megaraidli...@lsi.com) Adam Radford diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 2e2fcb2..bc7adcf 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -33,9 +33,9 @@ /* * MegaRAID SAS Driver meta data */ -#define MEGASAS_VERSION06.803.01.00-rc1 -#define MEGASAS_RELDATEMar. 10, 2014 -#define MEGASAS_EXT_VERSIONMon. Mar. 10 17:00:00 PDT 2014 +#define MEGASAS_VERSION06.803.02.00-rc1 +#define MEGASAS_RELDATEJun. 19, 2014 +#define MEGASAS_EXT_VERSIONThu. Jun. 19 17:00:00 PDT 2014 /* * Device IDs diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 8f62b30..222bab2 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -18,7 +18,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA * * FILE: megaraid_sas_base.c - * Version : 06.803.01.00-rc1 + * Version : 06.803.02.00-rc1 * * Authors: LSI Corporation * Sreenivas Bagalkote -- 1.7.11.7 -- 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