Re: [PATCH] virtio-scsi: replace target spinlock with seqcount
Il 28/05/2014 03:48, Ming Lei ha scritto:
On Wed, May 28, 2014 at 12:57 AM, Paolo Bonzini pbonz...@redhat.com wrote:
Il 27/05/2014 18:50, Venkatesh Srinivas ha scritto:
Hi,
I think this patch has a small race involving just two commands:
1. The first command to a target is in virtscsi_pick_vq(), after
atomic_inc_return(tgt-tgt_lock)) but before write_seqcount_begin()
Good catch, thanks Venkatesh
And it doesn't happen in my test, so looks it won't be easy to trigger, :-)
No, it's basically impossible because the race window is very small and
the first command (INQUIRY or REPORT LUNS) is likely to be synchronous
anyway. But it's there anyway.
Specifically:
@@ -508,19 +507,33 @@ static struct virtio_scsi_vq
*virtscsi_pick_vq(struct virtio_scsi *vscsi,
unsigned long flags;
u32 queue_num;
-spin_lock_irqsave(tgt-tgt_lock, flags);
+local_irq_save(flags);
+if (atomic_inc_return(tgt-reqs) 1) {
+unsigned long seq;
+
+do {
+seq = read_seqcount_begin(tgt-tgt_seq);
+vq = tgt-req_vq;
+} while (read_seqcount_retry(tgt-tgt_seq, seq));
+} else {
A second virtscsi_pick_vq() here will read a stale or NULL tgt-req_vq.
The NULL case is true, indeed I was going to send a patch to initialize
tgt-req_vq but I have not tested it.
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index fc054935eb1f..f0b4cdbfceb0 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -618,14 +631,17 @@ static int virtscsi_abort(struct scsi_cmnd *sc)
static int virtscsi_target_alloc(struct scsi_target *starget)
{
+ struct Scsi_Host *sh = dev_to_shost(starget-dev.parent);
+ struct virtio_scsi *vscsi = shost_priv(sh);
+
struct virtio_scsi_target_state *tgt =
kmalloc(sizeof(*tgt), GFP_KERNEL);
if (!tgt)
return -ENOMEM;
seqcount_init(tgt-tgt_seq);
atomic_set(tgt-reqs, 0);
- tgt-req_vq = NULL;
+ tgt-req_vq = vscsi-req_vqs[0];
starget-hostdata = tgt;
return 0;
Paolo, do you need me to integrate this one into the patch? or
you will make it as one standalone?
I will integrate it myself after testing it.
Paolo
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/12] scsi/NCR5380: remove unused BOARD_NORMAL and BOARD_NCR53C400
On Tue, Mar 18, 2014 at 11:42:13AM +1100, Finn Thain wrote: BOARD_NORMAL is completely unused and BOARD_NCR53C400 is used only by g_NCR5380 internally. Remove the unused definitions. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/12] scsi/NCR5380: remove redundant HOSTS_C macro tests
On Tue, Mar 18, 2014 at 11:42:14AM +1100, Finn Thain wrote: HOSTS_C is always undefined. There is no hosts.c anymore. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] scsi/NCR5380: remove old CVS keywords
On Tue, Mar 18, 2014 at 11:42:15AM +1100, Finn Thain wrote: Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 04/12] scsi/NCR5380: use NCR5380_dprint() instead of NCR5380_print()
On Tue, Mar 18, 2014 at 11:42:16AM +1100, Finn Thain wrote: Only the NCR5380_dprint() macro should invoke the NCR5380_print() function. That's why NCR5380.c only defines the function #if NDEBUG. Use the standard macro. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 05/12] scsi/NCR5380: fix build failures when debugging is enabled
On Tue, Mar 18, 2014 at 11:42:17AM +1100, Finn Thain wrote: The change from cmd-target to cmd-device-id was apparently the purpose of commit a7f251228390e87d86c5e3846f99a455517fdd8e in kernel/git/tglx/history.git but some instances have been missed. Also fix the NDEBUG_LAST_WRITE_SENT and NDEBUG_ALL typo's. Also fix some format strings (%ul becomes %lu) that caused compiler warnings. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 06/12] scsi/NCR5380: fix dprintk macro usage and definition
On Tue, Mar 18, 2014 at 11:42:18AM +1100, Finn Thain wrote: There are three implementations of the core NCR5380 driver and three sets of debugging macro definitions. And all three implementations use the NCR5380.h header as well. Two of the definitions of the dprintk macro accept a variable argument list whereas the third does not. Standardize on the variable argument list. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 07/12] scsi/NCR5380: adopt NCR5380_dprint() and NCR5380_dprint_phase()
On Tue, Mar 18, 2014 at 11:42:19AM +1100, Finn Thain wrote: All NCR5380 drivers already include the NCR5380.h header. Better to adopt those macros rather than have three variations on them. Moreover, the macros in NCR5380.h are preferable anyway: the atari_NCR5380 and sun3_NCR5380 versions are inflexible. For example, they can't accomodate NCR5380_dprint(NDEBUG_MAIN | NDEBUG_QUEUES, ...) Replace the NCR_PRINT* macros from atari_NCR5380.h and sun3_NCR5380.h with the equivalent macros from NCR5380.h. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 08/12] scsi/NCR5380: adopt dprintk()
On Tue, Mar 18, 2014 at 11:42:20AM +1100, Finn Thain wrote: All NCR5380 drivers already include the NCR5380.h header. Better to adopt those macros rather than have three variations on them. Moreover, the macros in NCR5380.h are preferable because the atari_NCR5380 and sun3_NCR5380 versions are inflexible. For example, they can't accomodate dprintk(NDEBUG_MAIN | NDEBUG_QUEUES, ...) Replace the *_PRINTK macros from atari_NCR5380.h and sun3_NCR5380.h with the equivalent macros from NCR5380.h. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/12] scsi/NCR5380: remove unused macro definitions
On Tue, Mar 18, 2014 at 11:42:22AM +1100, Finn Thain wrote: Remove the unused (and divergent) debugging macro definitions from the sun3_NCR5380 and atari_NCR5380 drivers. These drivers have been converted to use the common macros in NCR5380.h. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 09/12] scsi/NCR5380: fix and standardize NDEBUG macros
On Tue, Mar 18, 2014 at 11:42:21AM +1100, Finn Thain wrote: All three NCR5380 core driver implementations share the same NCR5380.h header file so they need to agree on certain macro definitions. The flag bit used by the NDEBUG_MERGING macro in atari_NCR5380 and sun3_NCR5380 collides with the bit used by NDEBUG_LISTS. Moreover, NDEBUG_ABORT appears in NCR5380.c so it should be defined in NCR5380.h rather than in each of the many drivers using that core. An undefined NDEBUG_ABORT macro caused compiler errors and led to dodgy workarounds in the core driver that can now be removed. (See commits f566a576bca09de85bf477fc0ab2c8c96405b77b and 185a7a1cd79b9891e3c17abdb103ba1c98d6ca7a.) Move all of the NDEBUG_ABORT, NDEBUG_TAGS and NDEBUG_MERGING macro definitions into NCR5380.h where all the other NDEBUG macros live. Also, incorrect #ifdef NDEBUG becomes #if NDEBUG to fix the warning: drivers/scsi/mac_scsi.c: At top level: drivers/scsi/NCR5380.c:418: warning: 'NCR5380_print' defined but not used drivers/scsi/NCR5380.c:459: warning: 'NCR5380_print_phase' defined but not used The debugging code is now enabled when NDEBUG != 0. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 12/12] scsi/NCR5380: merge sun3_scsi_vme.c into sun3_scsi.c
On Tue, Mar 18, 2014 at 11:42:24AM +1100, Finn Thain wrote: The sun3 drivers suffer from a whole bunch of duplicated code. Fix this by following the g_NCR5380_mmio example. (Notionally, sun3_scsi relates to sun3_scsi_vme in the same way that g_NCR5380 relates to g_NCR5380_mmio.) Dead code is also removed: we now have working debug macros so SUN3_SCSI_DEBUG is undesirable. Dead code within #ifdef OLD_DMA is also dropped, consistent with sun3_scsi_vme.c. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 11/12] scsi/NCR5380: reduce depth of sun3_scsi nested includes
On Tue, Mar 18, 2014 at 11:42:23AM +1100, Finn Thain wrote: Move the #include NCR5380.h out of the sun3_scsi.h header file and into the driver .c files, like all the other NCR5380 drivers in the tree. This improves uniformity and reduces the depth of nested includes. The sequence of #include's, #define's and #if's no longer does my head in. Signed-off-by: Finn Thain fth...@telegraphics.com.au Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 1/4] block: fix BLKSECTGET ioctl when max_sectors is greater than USHRT_MAX
On Sun, May 25, 2014 at 09:43:33PM +0900, Akinobu Mita wrote: BLKSECTGET ioctl loads the request queue's max_sectors as unsigned short value to the argument pointer. So if the max_sector is greater than USHRT_MAX, the upper 16 bits of that is just discarded. In such case, USHRT_MAX is more preferable than the lower 16 bits of max_sectors. Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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 2/4] block: fix SG_[GS]ET_RESERVED_SIZE ioctl when max_sectors is huge
On Sun, May 25, 2014 at 09:43:34PM +0900, Akinobu Mita wrote: SG_GET_RESERVED_SIZE and SG_SET_RESERVED_SIZE ioctls access a reserved buffer in bytes as int type. The value needs to be capped at the request queue's max_sectors. But integer overflow is not correctly handled in the calculation when converting max_sectors from sectors to bytes. Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] scsi_debug: allow huge transfer length for read/write commands
On Sun, May 25, 2014 at 09:43:36PM +0900, Akinobu Mita wrote: This change enables to test read/write commands with huge transfer length such as 1GB. For example: # modprobe scsi_debug dev_size_mb=1024 clustering=1 opts=1 # cat /sys/block/$DEV/queue/max_hw_sectors_kb \ /sys/block/$DEV/queue/max_sectors_kb # fio --name=test --rw=write --bs=1g --size=1g --filename=/dev/$DEV \ --mem=mmaphuge --direct=1 The data type of max_sectors in scsi_host_template has been extended to unsigned int by the previous change. So we can increase it from 0x to 0x to allow such huge transfer length. Also, this increases sg_tablesize and max_segment_size, otherwise the maximum transfer length is limited to 64MB. (sg_tablesize * max_segment_size = 256 * 256KB) Signed-off-by: Akinobu Mita akinobu.m...@gmail.com Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] scsi: increase upper limit for max_sectors
On Sun, May 25, 2014 at 09:43:35PM +0900, Akinobu Mita wrote: max_sectors in struct Scsi_Host specifies maximum number of sectors allowed in a single SCSI command. The data type of max_sectors is unsigned short, so the maximum transfer length per SCSI command is limited to less than 256MB in 4096-bytes sector size. (0x * 4096) This commit increases the SCSI mid level's limitation for max_sectors upto the block layer's limitation for max_hw_sectors by extending the data type of max_sectors in struct Scsi_Host and scsi_host_template, so that SCSI lower level drivers can specify more than 0x. This change requires the scsi disk (sd) driver to handle the requests whose transfer length is more than 0x with READ_16 or WRITE_16. Also, this needs to prevent SG_GET_RESERVED_SIZE and SG_SET_RESERVED_SIZE ioctls for the scsi generic (sg) driver from integer overflow when converting max_sectors to bytes. Signed-off-by: Akinobu Mita akinobu.m...@gmail.com Looks good, but I think this should be three patches, one for the ioctl in sg, one for the command selection in sd, and one to change the field with in the scsi core. -- 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] acornscsi: remove linked command support
On Sat, May 24, 2014 at 12:13:53PM +0200, Paul Bolle wrote: The acornscsi driver was added in v2.1.88. It has always #undef-ed CONFIG_SCSI_ACORNSCSI_LINK near the top of acornscsi.c. And, just to be sure, it has also always triggered a preprocessor error if CONFIG_SCSI_ACORNSCSI_LINK was still defined. But, as far as I can see, it has never even been possible to set SCSI_ACORNSCSI_LINK through kconfig, or its predecessors, in the first place. Let's remove the code involved. Signed-off-by: Paul Bolle pebo...@tiscali.nl Looks good, Reviewed-by: Christoph Hellwig h...@lst.de And I have to disagree with James here, removing code that isn't even compiled always is an improvement, especially for an unmaintained driver. -- 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 1/1] drivers/scsi/aic7xxx/aic79xx_osm.c: replace kmalloc/memset by kzalloc
Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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] mpt2sas: Don't disable device twice at suspend.
On Fri, Apr 25, 2014 at 04:41:04PM -0400, Tyler Stachecki wrote: On suspend, _scsih_suspend calls mpt2sas_base_free_resources, which in turn calls pci_disable_device if the device is enabled prior to suspending. However, _scsih_suspend also calls pci_disable_device itself. Thus, in the event that the device is enabled prior to suspending, pci_disable_device will be called twice. This patch removes the duplicate call to pci_disable_device in _scsi_suspend as it is both unnecessary and results in a kernel oops. Signed-off-by: Tyler Stachecki tstac...@binghamton.edu Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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: [ANNOUNCE] scsi patch queue tree
I've pushed the following changes to the drivers-for-3.16 tree. I've there's anyting matching the rules that I did forget please resend and/or ping me. Benoit Taine (2): qla4xxx: Use kmemdup instead of kmalloc + memcpy qla2xxx: Use kmemdup instead of kmalloc + memcpy Dan Carpenter (1): qla2xxx: fix incorrect debug printk Dolev Raviv (4): scsi: ufs: query descriptor API scsi: ufs: device query status and size check scsi: ufs: Logical Unit (LU) command queue depth scsi: ufs: Fix queue depth handling for best effort cases Finn Thain (12): scsi/NCR5380: remove unused BOARD_NORMAL and BOARD_NCR53C400 scsi/NCR5380: remove redundant HOSTS_C macro tests scsi/NCR5380: remove old CVS keywords scsi/NCR5380: use NCR5380_dprint() instead of NCR5380_print() scsi/NCR5380: fix build failures when debugging is enabled scsi/NCR5380: fix dprintk macro usage and definition scsi/NCR5380: adopt NCR5380_dprint() and NCR5380_dprint_phase() scsi/NCR5380: adopt dprintk() scsi/NCR5380: fix and standardize NDEBUG macros scsi/NCR5380: remove unused macro definitions scsi/NCR5380: reduce depth of sun3_scsi nested includes scsi/NCR5380: merge sun3_scsi_vme.c into sun3_scsi.c Jayamohan Kallickal (8): be2iscsi: Fix retrieving MCCQ_WRB in non-embedded Mbox path be2iscsi: Fix exposing Host in sysfs after adapter initialization is complete be2iscsi: Fix interrupt Coalescing mechanism. be2iscsi: Fix TCP parameters while connection offloading. be2iscsi: Fix memory corruption in MBX path be2iscsi: Fix destroy MCC-CQ before MCC-EQ is destroyed be2iscsi: Fix processing cqe for cxn whose endpoint is freed be2iscsi: Bump the driver version Joe Handzik (1): hpsa: fix bad comparison of signed with unsigned in hpsa_update_scsi_devices Matthew Wilcox (7): mpt3sas: Remove uses of serial_number mpt3sas: Remove use of DEF_SCSI_QCMD mpt2sas: Remove uses of serial_number mpt2sas: Remove use of DEF_SCSI_QCMD mpt2sas: Add free smids to the head, not tail of list fusion: Add free msg frames to the head, not tail of list fusion: Remove use of DEF_SCSI_QCMD Sujit Reddy Thumma (6): scsi: ufs: fix endianness sparse warnings scsi: ufs: make undeclared functions static scsi: ufs: Fix broken task management command implementation scsi: ufs: Fix hardware race conditions while aborting a command scsi: ufs: Fix device and host reset methods scsi: ufs: Improve UFS fatal error handling drivers/message/fusion/mptbase.c |2 +- drivers/message/fusion/mptfc.c | 12 +- drivers/message/fusion/mptsas.c | 10 +- drivers/message/fusion/mptscsih.c|8 +- drivers/message/fusion/mptscsih.h|2 +- drivers/message/fusion/mptspi.c | 12 +- drivers/scsi/NCR5380.c | 137 +++-- drivers/scsi/NCR5380.h | 32 +- drivers/scsi/arm/cumana_1.c |3 - drivers/scsi/arm/oak.c |3 - drivers/scsi/atari_NCR5380.c | 162 +++--- drivers/scsi/atari_scsi.c| 22 +- drivers/scsi/atari_scsi.h| 93 --- drivers/scsi/be2iscsi/be.h | 11 + drivers/scsi/be2iscsi/be_cmds.h | 31 +- drivers/scsi/be2iscsi/be_iscsi.c | 12 +- drivers/scsi/be2iscsi/be_main.c | 93 ++- drivers/scsi/be2iscsi/be_main.h |7 +- drivers/scsi/be2iscsi/be_mgmt.c | 64 ++- drivers/scsi/be2iscsi/be_mgmt.h |2 + drivers/scsi/dtc.c |2 - drivers/scsi/g_NCR5380.c |4 - drivers/scsi/g_NCR5380.h |7 - drivers/scsi/hpsa.c |2 +- drivers/scsi/mac_scsi.c | 10 - drivers/scsi/mac_scsi.h | 10 - drivers/scsi/mpt2sas/mpt2sas_base.c |8 +- drivers/scsi/mpt2sas/mpt2sas_base.h |2 +- drivers/scsi/mpt2sas/mpt2sas_ctl.c |2 +- drivers/scsi/mpt2sas/mpt2sas_scsih.c | 24 +- drivers/scsi/mpt3sas/mpt3sas_base.h |2 +- drivers/scsi/mpt3sas/mpt3sas_ctl.c |2 +- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 24 +- drivers/scsi/pas16.h |3 - drivers/scsi/qla2xxx/qla_mbx.c |3 +- drivers/scsi/qla2xxx/qla_os.c|3 +- drivers/scsi/qla4xxx/ql4_os.c|7 +- drivers/scsi/sun3_NCR5380.c | 156 +++-- drivers/scsi/sun3_scsi.c | 238 +--- drivers/scsi/sun3_scsi.h | 199 +-- drivers/scsi/sun3_scsi_vme.c | 588 +-- drivers/scsi/t128.c |4 - drivers/scsi/t128.h |7 - drivers/scsi/ufs/ufs.h | 74 ++- drivers/scsi/ufs/ufshcd.c| 1032 +++--- drivers/scsi/ufs/ufshcd.h| 22 +- drivers/scsi/ufs/ufshci.h|
Re: [PATCHv2 0/5] Support 64-bit LUNs
James, can you take a look at this series? I would be great to get it merged for 3.16! -- 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_lib: replace ambiguous Unhandled error code messages.
On Mon, May 26, 2014 at 09:27:15PM +0200, Maurizio Lombardi wrote: How about simple not setting description at all for this case? It has already been proposed before but James didn't like the idea. http://markmail.org/message/dumujpz4gfp3s4fp#query:+page:1+mid:dumujpz4gfp3s4fp+state:results James never replied to Robs question. I can't think of any value add that Unhandled XYZ adds when we already get a usesul message from the same error. James, care to comment what'd you like to see printed here? -- 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: mvsas: add support for Supermicro AOC-SAS2LP-MV8
On Fri, May 16, 2014 at 02:06:42PM +0200, Jan Kasprzak wrote: any news with this patch? Will it be acked by you and submitted upstream? Thanks! Give me an Acked-by and I'll pull it in. -- 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] qla2xxx: remove stub qlt_check_srr_debug()
On Sat, May 24, 2014 at 04:03:12PM +0200, Paul Bolle wrote: qlt_check_srr_debug() was added in v3.5. It is a stub function unless CONFIG_QLA_TGT_DEBUG_SRR is defined. But CONFIG_QLA_TGT_DEBUG_SRR will never be defined, because the Kconfig symbol QLA_TGT_DEBUG_SRR was never added to the tree. Signed-off-by: Paul Bolle pebo...@tiscali.nl Looks good, Reviewed-by: Christoph Hellwig h...@lst.de -- 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] notify block layer when using temporary change to cache_type
On Wed, May 28, 2014 at 12:58:27PM +0800, Vaughan Cao wrote:
No, I just want to keep the modification as small as possible. Checked
the actions of sd_revalidate_disk() again, it seems no harm to call it
from here. Also actual actions are skipped in sd_read_cache_type() if
cache_override!=0, so I suppose your original plan is to jump to
sd_revalidate_disk() from here.
I think the right fix is to factor out a helper like:
static void sd_set_flush_flag(struct scsi_disk *sdkp)
{
unsigned flush = 0;
if (sdkp-WCE) {
flush |= REQ_FLUSH;
if (sdkp-DPOFUA)
flush |= REQ_FUA;
}
blk_queue_flush(sdkp-disk-queue, flush);
}
which can be called from sd_revalidate_disk and cache_type_store.
However, that way may not be acceptable after further code review.
Changing the sdkp-WCE,RCD will cause these real parameters of the
underlying device *lost*. In sd_shutdown() and sd_suspend_common(), we
need them to call sd_sync_cache() if necessary. I don't think this
action is avoidable, just for the performance issue to solve. And we
can't change the mode just by setting q-flush_flags while leave
sdkp-WCE,RCD untouched either, because cache_type_show() needs
sdkp-WCE,RCD to present the temporary config to userspace.
If the user overrides the cache type he should expect data not to be
flushed, so I don't really think this is na issue. Even if it is it
should be a separate issue as it already exists before your patch.
--
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: [target:for-next 20/20] drivers/scsi/virtio_scsi.c:531:48: error: dereferencing pointer to incomplete type
Il 28/05/2014 00:21, Nicholas A. Bellinger ha scritto: On Mon, 2014-05-26 at 13:30 -0400, Martin K. Petersen wrote: Nic == Nicholas A Bellinger n...@linux-iscsi.org writes: What about #ifdef'ing VIRTIO_SCSI_F_T10_PI support out if !CONFIG_BLK_DEV_INTEGRITY? Nic I figured it was slightly cleaner to enable BLK_DEV_INTEGRITY by Nic default when referencing struct blk_integrity (following what Nic IBLOCK does), than adding the equivalent #ifdef's.. Nic MKP, do you have a preference on this..? Well, I guess how important the virtio stuff is in the embedded space? In the block layer we have all these wrappers that allow things to Do The Right Thing when the BLK_DEV_INTEGRITY is disabled. I'm not entirely sure it worth the hassle to do the same for target and virtio. The memory savings aren't that big to begin with. And besides, the bip pointer field in struct bio is about to become generic with my impending copy offload patches anyway, In that case, I'll leave as is unless Paolo has an objection. No, that's fine. Paolo -- 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: mpt2sas: mpt2sas_base.c: Fix for possible null pointer dereference
- ioc_status = le16_to_cpu(mpi_reply-IOCStatus) MPI2_IOCSTATUS_MASK;
+ if (mpi_reply) {
+ ioc_status = le16_to_cpu(mpi_reply-IOCStatus)
MPI2_IOCSTATUS_MASK;
+ }
if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
ioc-port_enable_failed = 1;
ioc_status isn't initialized without the reply and used here as well
as later in the function. I think we'll need input from LSI or others
with the spec on what to do when we didn't get a reply.
--
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: pm8001: pm8001_hwi.c: Fix for possible null pointer dereference
On Sun, May 18, 2014 at 06:14:00PM +0200, Rickard Strandqvist wrote: There is otherwise a risk of a possible null pointer dereference. Was largely found by using a static code analysis program called cppcheck. I can't see how dev-lldd_dev would be NULL here. -- 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: pm8001: pm8001_hwi.c: Fix for possible null pointer dereference
On 05/28/2014 01:28 PM, Christoph Hellwig wrote: On Sun, May 18, 2014 at 06:14:00PM +0200, Rickard Strandqvist wrote: There is otherwise a risk of a possible null pointer dereference. Was largely found by using a static code analysis program called cppcheck. I can't see how dev-lldd_dev would be NULL here. The analysis program likely can't know that, the reason for this report it that dev-lldd_dev is tested for not being null several lines below - http://lxr.free-electrons.com/source/drivers/scsi/pm8001/pm8001_hwi.c#L4447 So the other option is to remove this^ test. -- 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 05/12] scsi/NCR5380: fix build failures when debugging is enabled
On Wed, 28 May 2014, Christoph Hellwig wrote: Looks good, Reviewed-by: Christoph Hellwig h...@lst.de Thanks for reviewing. There was a later submission (version 2) with changes made to three of these patches, [PATCH v2 05/12] scsi/NCR5380: fix build failures when debugging is enabled [PATCH v2 06/12] scsi/NCR5380: fix dprintk macro usage and definition [PATCH v2 09/12] scsi/NCR5380: fix and standardize NDEBUG macros And several patches in the v2 submission had some acked-by tags added. The v2 series is the one to merge (but I think James intends to do that). -- -- 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 05/12] scsi/NCR5380: fix build failures when debugging is enabled
On Wed, May 28, 2014 at 10:09:33PM +1000, Finn Thain wrote: [PATCH v2 05/12] scsi/NCR5380: fix build failures when debugging is enabled [PATCH v2 06/12] scsi/NCR5380: fix dprintk macro usage and definition [PATCH v2 09/12] scsi/NCR5380: fix and standardize NDEBUG macros And several patches in the v2 submission had some acked-by tags added. The v2 series is the one to merge (but I think James intends to do that). I've pulled this version into the for-3.16 drivers tree I'm collection for james: http://git.infradead.org/users/hch/scsi-queue.git/shortlog/refs/heads/drivers-for-3.16 git://git.infradead.org/users/hch/scsi-queue.git drivers-for-3.16 Can you send incremental changes if needed? -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] scsi: increase upper limit for max_sectors
Hi Christoph, Thanks for your review. 2014-05-28 19:39 GMT+09:00 Christoph Hellwig h...@infradead.org: Looks good, but I think this should be three patches, one for the ioctl in sg, one for the command selection in sd, and one to change the field with in the scsi core. OK, I'll break this into three patches and rearrange the patchset for next submission. -- 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] scsi/NCR5380: dprintk macro
This is the delta between the two submissions:
[PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure
and
[PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure
The macro definition changes were discussed on the mailing list during
review. The idea is to get the compiler to check the parameters of
disabled printk() calls so that the debugging code doesn't rot again.
Signed-off-by: Finn Thain fth...@telegraphics.com.au
---
I know the whitespace is wrong throughout these drivers but when I put
this patch series together it seemed best to defer the whitespace cleanup.
(I still intend to do that cleanup.)
diff -ru v1/drivers/scsi/NCR5380.h v2/drivers/scsi/NCR5380.h
--- v1/drivers/scsi/NCR5380.h 2014-05-29 00:07:28.525627086 +1000
+++ v2/drivers/scsi/NCR5380.h 2014-05-29 00:06:41.335557873 +1000
@@ -295,9 +295,10 @@
#define NDEBUG (0)
#endif
+#define dprintk(flg, fmt, ...) \
+ do { if ((NDEBUG) (flg)) pr_debug(fmt, ## __VA_ARGS__); } while (0)
+
#if NDEBUG
-#define dprintk(flg, fmt, args...) \
- do { if ((NDEBUG) (flg)) pr_debug(fmt, ## args); } while (0)
#define NCR5380_dprint(flg, arg) \
do { if ((NDEBUG) (flg)) NCR5380_print(arg); } while (0)
#define NCR5380_dprint_phase(flg, arg) \
@@ -305,7 +306,6 @@
static void NCR5380_print_phase(struct Scsi_Host *instance);
static void NCR5380_print(struct Scsi_Host *instance);
#else
-#define dprintk(flg, fmt, args...) do {} while (0)
#define NCR5380_dprint(flg, arg) do {} while (0)
#define NCR5380_dprint_phase(flg, arg) do {} while (0)
#endif
diff -ru v1/drivers/scsi/sun3_NCR5380.c v2/drivers/scsi/sun3_NCR5380.c
--- v1/drivers/scsi/sun3_NCR5380.c 2014-05-29 00:07:28.525627086 +1000
+++ v2/drivers/scsi/sun3_NCR5380.c 2014-05-29 00:06:41.37902 +1000
@@ -1006,12 +1006,8 @@
for (tmp = (struct scsi_cmnd *) hostdata-issue_queue,
prev = NULL; tmp; prev = tmp, tmp = NEXT(tmp) ) {
-#if (NDEBUG NDEBUG_LISTS)
if (prev != tmp)
- printk(MAIN tmp=%p target=%d busy=%d lun=%d\n,
- tmp, tmp-target, hostdata-busy[tmp-target],
- tmp-lun);
-#endif
+ dprintk(NDEBUG_LISTS, MAIN tmp=%p target=%d
busy=%d lun=%d\n, tmp, tmp-device-id, hostdata-busy[tmp-device-id],
tmp-device-lun);
/* When we find one, remove it from the issue queue. */
/* ++guenther: possible race with Falcon locking */
if (
--
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] acornscsi: remove linked command support
On Wed, May 28, 2014 at 06:26:44PM +0400, James Bottomley wrote: On Wed, 2014-05-28 at 03:41 -0700, Christoph Hellwig wrote: Looks good, Reviewed-by: Christoph Hellwig h...@lst.de And I have to disagree with James here, removing code that isn't even compiled always is an improvement, especially for an unmaintained driver. Well, as I said, this is in theory a maintained driver, so just get an ack from Russell and this debate is moot. I don't see any harm in removing this - I don't think I've owned any SCSI devices which supported linked commands, which is why I never put much effort into it. As no one else seems to care about it (given the number of years it's been left alone), removing the linked command stuff is fine. Acked-by: Russell King rmk+ker...@arm.linux.org.uk -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- 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: [ANNOUNCE] scsi patch queue tree
On Wed, May 28, 2014 at 10:37:31AM -0500, Mike Christie wrote: On 05/28/2014 05:54 AM, Christoph Hellwig wrote: be2iscsi: Fix processing cqe for cxn whose endpoint is freed I didn't look at your tree, but when this patch was posted on the list I think it had a bug. http://www.spinics.net/lists/linux-scsi/msg74191.html I read through it and saw your comment, but so far it seems the patch and especially the series does more good than harm. I'll ping the author again to get a fix for the issue you pointed out - I was planning to do that anwyay. -- 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 7/8] be2iscsi: Fix processing cqe for cxn whose endpoint is freed
On Wed, May 07, 2014 at 05:18:38PM -0500, Mike Christie wrote: It looks like if that race is possible then we could also free the ep while you are accessing right? I think you would need to get a ref to the ep. What command/function tells the card to stop sending the driver events/notifications/ios for that connection? Is it beiscsi_close_conn or mgmt_invalidate_connection? Jay, can you look into this issue please? I've applied the series to the scsi queue for now, but I'd really prefer to see this addressed ASAP. -- 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/NCR5380: dprintk macro
On Thu, May 29, 2014 at 12:43:43AM +1000, Finn Thain wrote: This is the delta between the two submissions: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure and [PATCH v2 00/12] scsi/NCR5380: fix debugging macros and #include structure The macro definition changes were discussed on the mailing list during review. The idea is to get the compiler to check the parameters of disabled printk() calls so that the debugging code doesn't rot again. Signed-off-by: Finn Thain fth...@telegraphics.com.au Thanks a lot Finn! I know the whitespace is wrong throughout these drivers but when I put this patch series together it seemed best to defer the whitespace cleanup. (I still intend to do that cleanup.) Yes, I ignored the whitespace warnings in the debug macro cleanups. I fixed them up for the newly added code in the sun3 driver merge. -- 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: [ANNOUNCE] scsi patch queue tree
On 5/28/2014 4:24 PM, Christoph Hellwig wrote: I've pushed the following changes to the drivers-for-3.16 tree. I've there's anyting matching the rules that I did forget please resend and/or ping me. Benoit Taine (2): qla4xxx: Use kmemdup instead of kmalloc + memcpy qla2xxx: Use kmemdup instead of kmalloc + memcpy Dan Carpenter (1): qla2xxx: fix incorrect debug printk Dolev Raviv (4): scsi: ufs: query descriptor API scsi: ufs: device query status and size check scsi: ufs: Logical Unit (LU) command queue depth scsi: ufs: Fix queue depth handling for best effort cases The above 4 patches are just posted to mailing lists with no review/ack's yet. I believe it still went in because the sender has modified the author name to himself and signed-off by is present by the original author which worked as a positive review for your rules. I have asked the sender to check why the original author name is changed. Meanwhile, I believe these patches should get some more time for review. -- Regards, Sujit -- 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: [ANNOUNCE] scsi patch queue tree
On Wed, May 28, 2014 at 09:31:14PM +0530, Sujit Reddy Thumma wrote: The above 4 patches are just posted to mailing lists with no review/ack's yet. I believe it still went in because the sender has modified the author name to himself and signed-off by is present by the original author which worked as a positive review for your rules. I have asked the sender to check why the original author name is changed. Meanwhile, I believe these patches should get some more time for review. Yes, I did count this as a positive review for now. I can drop the patches for now. -- 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, repost] mptfusion: fix msgContext in mptctl_hp_hostinfo
Hi, without this patch the istwiRWRequest-MsgContext is always set to zero, this patch saves the MsgContext in a msgcontext variable and then restores the value. Thanks to David Jeffery who found the issue and did the analysis. Signed-off-by: Tomas Henzl the...@redhat.com Acked-by: Desai, Kashyap kashyap.de...@lsi.com --- drivers/message/fusion/mptctl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c index dcc8385..8a050e8 100644 --- a/drivers/message/fusion/mptctl.c +++ b/drivers/message/fusion/mptctl.c @@ -2432,9 +2432,9 @@ mptctl_hp_hostinfo(unsigned long arg, unsigned int data_size) int rc, cim_rev; ToolboxIstwiReadWriteRequest_t *IstwiRWRequest; MPT_FRAME_HDR *mf = NULL; - MPIHeader_t *mpi_hdr; unsigned long timeleft; int retval; + u32 msgcontext; /* Reset long to int. Should affect IA64 and SPARC only */ @@ -2581,11 +2581,11 @@ mptctl_hp_hostinfo(unsigned long arg, unsigned int data_size) } IstwiRWRequest = (ToolboxIstwiReadWriteRequest_t *)mf; - mpi_hdr = (MPIHeader_t *) mf; + msgcontext = IstwiRWRequest-MsgContext; memset(IstwiRWRequest,0,sizeof(ToolboxIstwiReadWriteRequest_t)); + IstwiRWRequest-MsgContext = msgcontext; IstwiRWRequest-Function = MPI_FUNCTION_TOOLBOX; IstwiRWRequest-Tool = MPI_TOOLBOX_ISTWI_READ_WRITE_TOOL; - IstwiRWRequest-MsgContext = mpi_hdr-MsgContext; IstwiRWRequest-Flags = MPI_TB_ISTWI_FLAGS_READ; IstwiRWRequest-NumAddressBytes = 0x01; IstwiRWRequest-DataLength = cpu_to_le16(0x04); -- 1.8.3.1 -- 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: [ANNOUNCE] scsi patch queue tree
On 05/28/2014 10:50 AM, Christoph Hellwig wrote: On Wed, May 28, 2014 at 10:37:31AM -0500, Mike Christie wrote: On 05/28/2014 05:54 AM, Christoph Hellwig wrote: be2iscsi: Fix processing cqe for cxn whose endpoint is freed I didn't look at your tree, but when this patch was posted on the list I think it had a bug. http://www.spinics.net/lists/linux-scsi/msg74191.html I read through it and saw your comment, but so far it seems the patch and especially the series does more good than harm. Did you see these patchsets: [PATCH 00/10] qla4xxx: 5.04.00-k5: Updates for scsi misc branch and [PATCH 0/6] qla4xxx: 5.04.00-k6: Updates for scsi misc branch ? I think they should be ok for your tree if you did not see any issues with them. -- 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: [ANNOUNCE] scsi patch queue tree
On Wed, May 28, 2014 at 11:04:31AM -0500, Mike Christie wrote: Did you see these patchsets: [PATCH 00/10] qla4xxx: 5.04.00-k5: Updates for scsi misc branch and [PATCH 0/6] qla4xxx: 5.04.00-k6: Updates for scsi misc branch ? I think they should be ok for your tree if you did not see any issues with them. I did merge them 10 days ago. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] scsi: increase upper limit for max_sectors
-Original Message-
From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
ow...@vger.kernel.org] On Behalf Of Akinobu Mita
Sent: Sunday, 25 May, 2014 7:44 AM
To: linux-scsi@vger.kernel.org
Cc: Akinobu Mita; Jens Axboe; James E.J. Bottomley; Douglas Gilbert
Subject: [PATCH 3/4] scsi: increase upper limit for max_sectors
...
- } else if (sdp-use_16_for_rw) {
+ } else if ((this_count 0x) || sdp-use_16_for_rw) {
Suggestion: reorder those so the unlikely case of a huge
this_count is on the right side of the || operator.
--
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] [SCSI] Fix T10-DIF-TYPE3 get_tag handler for multi-sector bios
The combined tag size in this case is 6 bytes; fix the loop.
Signed-off-by: Andreas Gruenbacher andreas.gruenbac...@gmail.com
---
drivers/scsi/sd_dif.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index a7a691d..d657a8c 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -264,7 +264,7 @@ static void sd_dif_type3_get_tag(void *prot, void *tag_buf,
unsigned int sectors
u8 *tag = tag_buf;
unsigned int i, j;
- for (i = 0, j = 0 ; i sectors ; i++, j += 2, sdt++) {
+ for (i = 0, j = 0 ; i sectors ; i++, j += 6, sdt++) {
tag[j] = (sdt-app_tag 0xff00) 8;
tag[j+1] = sdt-app_tag 0xff;
tag[j+2] = (sdt-ref_tag 0xff00) 24;
--
1.8.3.1
--
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] acornscsi: remove linked command support
On Sun, 2014-05-25 at 11:42 +0400, James Bottomley wrote: On Sat, 2014-05-24 at 15:16 +0200, Paul Bolle wrote: On Sat, 2014-05-24 at 16:13 +0400, James Bottomley wrote: Wait, no, that's not a good idea. We leave obsolete drivers to bitrot. Particularly we try not to touch them unless we have to because there might be a few people still using them and the more we tamper, the greater the risk that something gets broken. Which is also a way to notice whether people still use those obsolete drivers. Really, no. We don't deliberately break old drivers to see if anyone notices. Usually the feedback loop is months to years for the long tail to notice and by that time fixing the problem becomes a real pain if you allow driver churn. Of course I wasn't advocating deliberately breaking old drivers. But it's easy to get annoyed by that short remark. It would have been better if I hadn't made it. Especially because I didn't also point out, as Cristoph did, that the code I want removed doesn't get compiled. So removing it can, by definition I'd say, not break that old driver. We keep old drivers that compile until there's a problem caused usually by something like API changes. On that principle, since there's no real reason to remove the code, (Unless one carries the hope that any check, treewide, for a CONFIG_* macro can be linked to a proper Kconfig symbol.) The check can be fixed. If you look at what Fengguang Wu does, he has a list of expected problems which he diffs. Don't churn the tree to match the checker, make the checker match the tree. Sure. See my recent patch to scripts/headers_check.pl, which does just that. But before one special cases a certain hit for a checker one needs to know that this hit really can't or won't be fixed. And in order to know that one needs to at least try to fix it first. it should stay ... until the whole driver bitrots to the extent that we can no-longer compile it. I've run into this depreciation policy before. With aic7xxx_old (which I eventually convinced Fedora to disable, a few relases before it was removed from the tree). With aic94xx (which I also convinced Fedora to disable). I also tried multiple times to shut up advansys' compile time[1]. It seems SCSI might risk not to notice their bitrot, because eventually everybody stops compiling these obsolete drivers, leaving SCSI without feedback on their state. Why would we care? If it compiles that's fine, it's not causing a problem and it might just be useful to somebody. Fair point: having code that no one uses doesn't cost a lot. The time obsolete drivers cause problems is tree or subsystem wide API changes. Then we look at the amount of work required and sometimes remove them or do hack fixes. Anyhow, SCSI seems to be the only subsystem that uses this subcategory of not-really-maintained drivers. Other subsystems appear to allow anything to be fixed, even trivialities, which are what I tend to fix, and only stop allowing fixes if the drivers involved are going to be removed anyway. But maybe I just never ran into that category in other subsystems. Try ide ... they have the same policy. I never really touched IDE. That might explain why I only ran into this issue with SCSI. Try to understand the reason: we have a long tail of people using obsolete systems who we try not to break. Any change to an unmaintained driver which can't be tested risks that ... and I'm the one who would have to try to sort out the problem when it's noticed, hence the caution. If we allow trivial churn, by the time the breakage is noticed (usually months to years later), the driver will have picked up a ton of changes and finding the problem one becomes really hard. So unmaintained drivers get a default deep freeze maintenance policy. Thanks for taking the time to explain this to me. I appreciate that. This is the first time, I think, that I've seen you explain that policy. (I might have missed earlier explanations to other people.) Now I might not entirely agree with you, but it does help to know where you're coming from. Thanks, Paul Bolle -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 0/5] lpfc 10.2.8001.0: Update lpfc to revision 10.2.8001.0
looks good Signed-By: Dick Kennedy dick.kenn...@emulex.com -- 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 39/42] qla2xxx: ABTS cause double free of qla_tgt_cmd +.
Nicholas,
At this point we can forgo this patch to stable tree. There are
additional tweaks in this path during testing. We will update the stable
tree with new findings.
Thanks.
Regards,
Quinn Tran
On 5/22/14 8:00 PM, Nicholas A. Bellinger n...@linux-iscsi.org wrote:
Hi Saurav + Quinn,
Just curious if this fix needs to be CC'ed to stable as well, or if it's
something that is only triggered with the preceding T10 DIF support
patch in place..?
--nab
On Fri, 2014-04-11 at 16:54 -0400, Saurav Kashyap wrote:
From: Quinn Tran quinn.t...@qlogic.com
Fix double free problem within qla2xxx driver where
current code prematurely free qla_tgt_cmd while firmware
still has the command. When firmware release the command
after abort, the code attempt a second free as part of
command completion processing.
When TCM start the free process, NULL pointer was hit.
--
WARNING: CPU: 8 PID: 43613 at lib/list_debug.c:62
__list_del_entry+0x82/0xd0()
list_del corruption. next-prev should be 88082b5cfb08, but was
6b6b6b6b6b6b6b6b
CPU: 8 PID: 43613 Comm: kworker/8:0 Tainted: GF W O
3.13.0-rc3-nab_t10dif+ #6
Hardware name: HP ProLiant DL380p Gen8, BIOS P70 08/20/2012
Workqueue: events cache_reap
003e 88081b2e3c78 815a051f 003e
88081b2e3cc8 88081b2e3cb8 8104fc2c
88082b5cfb00 88081c788d00 88082b5d7200 88082b5d3080
Call Trace:
[815a051f] dump_stack+0x49/0x62
[8104fc2c] warn_slowpath_common+0x8c/0xc0
[8104fd16] warn_slowpath_fmt+0x46/0x50
[812b6592] __list_del_entry+0x82/0xd0
[8106d48c] process_one_work+0x12c/0x510
[8106d4d3] ? process_one_work+0x173/0x510
[8106ebdf] worker_thread+0x11f/0x3a0
[8106eac0] ? manage_workers+0x170/0x170
[81074f26] kthread+0xf6/0x120
[8109f103] ? __lock_release+0x133/0x1b0
[81074e30] ? __init_kthread_worker+0x70/0x70
[815aec2c] ret_from_fork+0x7c/0xb0
[81074e30] ? __init_kthread_worker+0x70/0x70
---[ end trace dfc05c3f7caf8ebe ]---
BUG: unable to handle kernel NULL pointer dereference at
0008
IP: [8106d391] process_one_work+0x31/0x510
---
Signed-off-by: Quinn Tran quinn.t...@qlogic.com
Signed-off-by: Giridhar Malavali giridhar.malav...@qlogic.com
Signed-off-by: Saurav Kashyap saurav.kash...@qlogic.com
---
drivers/scsi/qla2xxx/qla_target.c | 29 ++---
1 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_target.c
b/drivers/scsi/qla2xxx/qla_target.c
index f24d44c..b1d10f9 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -2681,7 +2681,18 @@ static void qlt_send_term_exchange(struct
scsi_qla_host *vha,
rc = __qlt_send_term_exchange(vha, cmd, atio);
spin_unlock_irqrestore(vha-hw-hardware_lock, flags);
done:
-if (rc == 1) {
+/*
+ * Terminate exchange will tell fw to release any active CTIO
+ * that's in FW posession and cleanup the exchange.
+ *
+ * cmd-state == QLA_TGT_STATE_ABORTED means CTIO is still
+ * down at FW. Free the cmd later when CTIO comes back later
+ * w/aborted(0x2) status.
+ *
+ * cmd-state != QLA_TGT_STATE_ABORTED means CTIO is already
+ * back w/some err. Free the cmd now.
+ */
+if ((rc == 1) (cmd-state != QLA_TGT_STATE_ABORTED)) {
if (!ha_locked !in_interrupt())
msleep(250); /* just in case */
@@ -2689,6 +2700,7 @@ done:
qlt_unmap_sg(vha, cmd);
vha-hw-tgt.tgt_ops-free_cmd(cmd);
}
+return;
}
void qlt_free_cmd(struct qla_tgt_cmd *cmd)
@@ -2906,6 +2918,7 @@ static void qlt_do_ctio_completion(struct
scsi_qla_host *vha, uint32_t handle,
case CTIO_LIP_RESET:
case CTIO_TARGET_RESET:
case CTIO_ABORTED:
+/* driver request abort via Terminate exchange */
case CTIO_TIMEOUT:
case CTIO_INVALID_RX_ID:
/* They are OK */
@@ -2974,9 +2987,18 @@ static void qlt_do_ctio_completion(struct
scsi_qla_host *vha, uint32_t handle,
break;
}
-if (cmd-state != QLA_TGT_STATE_NEED_DATA)
+
+/* cmd-state == QLA_TGT_STATE_ABORTED means
+ * cmd is already aborted/terminated, we don't
+ * need to terminate again. The exchange is already
+ * cleaned up/freed at FW level. Just cleanup at driver
+ * level.
+ */
+if ((cmd-state != QLA_TGT_STATE_NEED_DATA)
+(cmd-state != QLA_TGT_STATE_ABORTED)) {
if (qlt_term_ctio_exchange(vha, ctio, cmd, status))
return;
+}
}
skip_term:
@@ -3007,7 +3029,8 @@ skip_term:
not return
[PATCH 1/1] drivers/scsi/aic7xxx/aic7xxx_core.c: replace 3 kmalloc/memset 0 by kzalloc
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: Hannes Reinecke h...@suse.de
Signed-off-by: Fabian Frederick f...@skynet.be
---
drivers/scsi/aic7xxx/aic7xxx_core.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c
b/drivers/scsi/aic7xxx/aic7xxx_core.c
index 10172a3..7f34d25 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_core.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
@@ -4464,10 +4464,9 @@ ahc_softc_init(struct ahc_softc *ahc)
ahc-pause = ahc-unpause | PAUSE;
/* XXX The shared scb data stuff should be deprecated */
if (ahc-scb_data == NULL) {
- ahc-scb_data = kmalloc(sizeof(*ahc-scb_data), GFP_ATOMIC);
+ ahc-scb_data = kzalloc(sizeof(*ahc-scb_data), GFP_ATOMIC);
if (ahc-scb_data == NULL)
return (ENOMEM);
- memset(ahc-scb_data, 0, sizeof(*ahc-scb_data));
}
return (0);
@@ -4780,10 +4779,9 @@ ahc_init_scbdata(struct ahc_softc *ahc)
SLIST_INIT(scb_data-sg_maps);
/* Allocate SCB resources */
- scb_data-scbarray = kmalloc(sizeof(struct scb) * AHC_SCB_MAX_ALLOC,
GFP_ATOMIC);
+ scb_data-scbarray = kzalloc(sizeof(struct scb) * AHC_SCB_MAX_ALLOC,
GFP_ATOMIC);
if (scb_data-scbarray == NULL)
return (ENOMEM);
- memset(scb_data-scbarray, 0, sizeof(struct scb) * AHC_SCB_MAX_ALLOC);
/* Determine the number of hardware SCBs and initialize them */
@@ -7558,14 +7556,13 @@ ahc_handle_en_lun(struct ahc_softc *ahc, struct cam_sim
*sim, union ccb *ccb)
return;
}
}
- lstate = kmalloc(sizeof(*lstate), GFP_ATOMIC);
+ lstate = kzalloc(sizeof(*lstate), GFP_ATOMIC);
if (lstate == NULL) {
xpt_print_path(ccb-ccb_h.path);
printk(Couldn't allocate lstate\n);
ccb-ccb_h.status = CAM_RESRC_UNAVAIL;
return;
}
- memset(lstate, 0, sizeof(*lstate));
status = xpt_create_path(lstate-path, /*periph*/NULL,
xpt_path_path_id(ccb-ccb_h.path),
xpt_path_target_id(ccb-ccb_h.path),
--
1.9.1
--
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] drivers/scsi/aic7xxx/aic79xx_core.c: replace kmalloc/memset 0 by kzalloc
Cc: Hannes Reinecke h...@suse.de
Cc: James E.J. Bottomley jbottom...@parallels.com
Signed-off-by: Fabian Frederick f...@skynet.be
---
drivers/scsi/aic7xxx/aic79xx_core.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c
b/drivers/scsi/aic7xxx/aic79xx_core.c
index 0bcacf7..9ce383c 100644
--- a/drivers/scsi/aic7xxx/aic79xx_core.c
+++ b/drivers/scsi/aic7xxx/aic79xx_core.c
@@ -10437,14 +10437,13 @@ ahd_handle_en_lun(struct ahd_softc *ahd, struct
cam_sim *sim, union ccb *ccb)
return;
}
}
- lstate = kmalloc(sizeof(*lstate), GFP_ATOMIC);
+ lstate = kzalloc(sizeof(*lstate), GFP_ATOMIC);
if (lstate == NULL) {
xpt_print_path(ccb-ccb_h.path);
printk(Couldn't allocate lstate\n);
ccb-ccb_h.status = CAM_RESRC_UNAVAIL;
return;
}
- memset(lstate, 0, sizeof(*lstate));
status = xpt_create_path(lstate-path, /*periph*/NULL,
xpt_path_path_id(ccb-ccb_h.path),
xpt_path_target_id(ccb-ccb_h.path),
--
1.9.1
--
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
Open-FCoE Maintainer Transition
Hello community, It is with mixed feelings that I send this mail to state that I am transitioning maintainership of Open-FCoE. I'm still working on Linux solutions, but now I'm focusing on SDN/NfV technologies. I'll still be around and I'll continue to help where I can. Vasu Dev will take over the Open-FCoE project. He's been a developer on the project since the beginning and understands the code base very well. I'll send a patch to update MAINTAINERS shortly. It has been a pleasure working with everyone, Thanks, //Rob -- 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: [PATCHv2 0/5] Support 64-bit LUNs
On Tue, 2014-05-20 at 13:03 +0200, Hannes Reinecke wrote: Hi all, this patchset updates the SCSI stack to support full 64-bit LUNs. The first patch is a simple fix; the next patch updates the sequential scan logic to be compliant with SPC. The third patch addresses a firmware issue with earlier qla2xxx HBAs. The last two patches update the SCSI stack and all drivers to use 64-bit LUNs where appropriate. Two drivers have issues with 64bit LUNs: - The qla2xxx driver uses a 32-bit LUN value for TMFs. But as the driver uses a max_lun value from 0x we should be safe for the time being. - The zfcp driver uses a 32-bit LUN for debug records; the record format would need to be updated to cope with 64-bit LUNs. But again, this driver uses 0x for max_lun, so it doesn't do any harm. The other changes have been pretty straightforward. ... So, the patches look like they will work fine... I think we should make sure the comments/documentation match the code, though. For example: - Remove this reference in Documentation/scsi/tmscsim.txt: The default for LUN Check depends on CONFIG_SCSI_MULTI_LUN. - include/scsi/scsi_host.h comment needs adjusting due to fields moved in struct Scsi_Host: /* * These three parameters can be used to allow for wide scsi, * and for host adapters that support multiple busses * The first two should be set to 1 more than the actual max id * or lun (e.g. 8 for SCSI parallel systems). */ unsigned int max_channel; unsigned int max_id; u64 max_lun; - int_to_scsilun() and scsilun_to_int() no longer take/return an int. Yes, the names are exported and that is a pain but it seems to me that this is just asking for an inadvertent truncation of the value. Also, the following comment is no longer true: * Note: the scsilun_to_int() routine does not truly handle all * 8bytes of the lun value. This functions restores only as much * as was set by the routine. So, is ibmvfc the only driver that actually supports a LUN value that is 32 bits? Or do we have other drivers that are ready to use this? Regardless, I think we should move forward with these changes. My concerns could easily be addressed with a subsequent patch. Reviewed-by: Ewan D. Milne emi...@redhat.com -- 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: Make SCSI error handler code easier to understand
On Mon, 26 May 2014 17:12:27 +0200 Bart Van Assche bvanass...@acm.org wrote: Every now and then someone asks how it is avoided that the SCSI error handler and the SCSI completion handler are invoked concurrently for the same SCSI command. Hence this patch series that should make the SCSI error handler code a little easier to understand. Hi Bart, We talked about REQ_ATOM_COMPLETE a while back (so I may be one of those people who periodically ask about SCSI error handling / completion), and I just thought I'd chime in here to clarify the scenario we were worried about. Before d555a2ab [SCSI] Fix spurious request sense in error handling we saw the following sequence of events: [ 1394.345991] sd 2:0:1:0: [sdw] Done: [ 1394.361790] TIMEOUT [ 1394.374148] sd 2:0:1:0: [sdw] [ 1394.388775] Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 1394.409068] sd 2:0:1:0: [sdw] CDB: [ 1394.424782] Read(10): 28 00 00 00 00 18 00 00 08 00 [ 1394.443772] sd 2:0:1:0: [sdw] scmd 88104dce02c0 abort scheduled [ 1394.474026] sd 2:0:1:0: [sdw] aborting command 88104dce02c0 [ 1394.573338] qla2xxx [:46:00.0]-801c:2: Abort command issued nexus=2:1:0 -- 1 2002. [ 1394.609313] sd 2:0:1:0: [sdw] scmd 88104dce02c0 retry aborted command [ 1425.397434] sd 2:0:1:0: [sdw] Done: [ 1425.417802] TIMEOUT [ 1425.431914] sd 2:0:1:0: [sdw] [ 1425.448247] Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK [ 1425.470736] sd 2:0:1:0: [sdw] CDB: [ 1425.488005] Read(10): 28 00 00 00 00 18 00 00 08 00 [ 1425.508472] sd 2:0:1:0: [sdw] scmd 88104dce02c0 previous abort failed [ 1425.533299] scsi_eh_2: waking up 0/1/1 [ 1425.551189] sd 2:0:1:0: scsi_eh_prt_fail_stats: cmds failed: 1, cancel: 0 [ 1425.575897] Total of 1 commands on 1 devices require eh work [ 1425.597970] sd 2:0:1:0: [sdw] scsi_eh_2: requesting sense ie, something like: CMD - .queuecommand ... timeout ABORT - .eh_abort_handler CMD (retry) - .queuecommand ... timeout ABORT - scsi_eh_scmd_add(scmd, SCSI_EH_CANCEL_CMD) ... into scsi_error_handler ... REQUEST SENSE - .queuecommand We eventually found our way into scsi_unjam_host, which *knows* that all commands on the bus have either completed, failed or timed out., and then scsi_send_eh_cmnd, which hijacks a SCSI command structure. Presumably the second comment follows from the first. But in the case of timeout, when was the LLDD informed to abort or forget about that scsi command structure? In our testing, it appeared that the LLDD's .queuecommand was called back-to-back with the same scsi_cmnd pointer. The driver in question was qla2xxx, who keeps a driver-private structure to scsi_cmnd mapping (assumed to be one-to-one) that got confused by this sequence of events. After d555a2ab, in this scenario scsi_unjam_host skips the request sense and escalates to the next level (scsi_eh_abort_cmds), skipping straight to the abort as we would have expected... This avoids the scsi_cmnd hijack... but was it ever safe to do so in the first place? Or should LLDD expect to gracefully handle the same scsi_cmnd pointer coming into .queuecommand w/o an intermediate abort or other scsi_host_template callback? Regards, -- Joe -- 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] Transition maintainership to Vasu.
Acked-by: Vasu Dev vasu@intel.com Signed-off-by: Robert Love robert.w.l...@intel.com --- MAINTAINERS |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 8ccf31c..c9afcf3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3539,7 +3539,7 @@ F:Documentation/fault-injection/ F: lib/fault-inject.c FCOE SUBSYSTEM (libfc, libfcoe, fcoe) -M: Robert Love robert.w.l...@intel.com +M: Vasu Dev vasu@intel.com L: fcoe-de...@open-fcoe.org W: www.Open-FCoE.org S: Supported -- 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: [Open-FCoE] Open-FCoE Maintainer Transition
Robert Thanks for your help and support with open-fcoe project and I really appreciate your help all along. Hope you have a very good luck with your new assignment. Thanks, Prasad Gondi Tech Lead (Storage Protocols) VMware Inc - Original Message - From: Robert Love robert.w.l...@intel.com To: fcoe-de...@open-fcoe.org, linux-scsi@vger.kernel.org Sent: Wednesday, May 28, 2014 12:46:24 PM Subject: [Open-FCoE] Open-FCoE Maintainer Transition Hello community, It is with mixed feelings that I send this mail to state that I am transitioning maintainership of Open-FCoE. I'm still working on Linux solutions, but now I'm focusing on SDN/NfV technologies. I'll still be around and I'll continue to help where I can. Vasu Dev will take over the Open-FCoE project. He's been a developer on the project since the beginning and understands the code base very well. I'll send a patch to update MAINTAINERS shortly. It has been a pleasure working with everyone, Thanks, //Rob ___ fcoe-devel mailing list fcoe-de...@open-fcoe.org https://urldefense.proofpoint.com/v1/url?u=http://lists.open-fcoe.org/mailman/listinfo/fcoe-develk=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0Ar=cSVqq0BhuSdMTew6%2FkKoKwRFLDyOjAnPCs48zqK9Xmk%3D%0Am=SPIp1OA%2F5bb8myYx4IUgylG0y%2BOkRIVn8gX8NHlTphc%3D%0As=c280151d67348fbdc0a22a8600709a80d28f7173127009fff147841bba25689e -- 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
Data integrity update
Here's an update to the block layer and SCSI data integrity code. There are a whole bunch of cleanups, some as a result of the work that Kent did to the block layer a while back. A bunch of dead code is removed, mainly the tagging functionality that nobody ended up using. There's also some prep work for the copy offload patches (separate series) that like the integrity code rely on being able to store additional information in each bio. The new functionality introduced is: - Exposing whether disks are formatted with PI in the bdev integrity profile so we can reliably distinguish between DIX Type 0 and DIX Type 1 - Allowing the choice of checksum and tag checking to be specified on a per-I/O basis - Data integrity specific error numbers - Moving the T10 protection information specifics to lib/ so that non-sd drivers can benefit from them - Adding support for a subset of DIX1.1 to the scsi_cmnd flags. These flags instruct the HBA drivers how to set up the protected transfer Documentation/ABI/testing/sysfs-block |9 Documentation/block/data-integrity.txt | 54 - block/Kconfig |1 block/bio-integrity.c | 273 ++ block/blk-core.c | 12 + block/blk-integrity.c | 102 ++--- block/blk-merge.c |6 drivers/md/dm-mpath.c |9 drivers/scsi/Kconfig |2 drivers/scsi/scsi_lib.c| 30 ++ drivers/scsi/sd.c | 56 - drivers/scsi/sd.h |4 drivers/scsi/sd_dif.c | 337 + include/linux/bio.h| 62 -- include/linux/blk_types.h | 14 - include/linux/blkdev.h | 54 ++--- include/linux/crc-t10dif.h |5 include/linux/t10-pi.h | 28 ++ include/scsi/scsi_cmnd.h | 29 ++ include/uapi/asm-generic/errno.h | 11 + lib/Kconfig|7 lib/Makefile |2 lib/t10-pi.c | 164 23 files changed, 627 insertions(+), 644 deletions(-) -- 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
[PATCH 01/14] block: Get rid of bdev_integrity_enabled()
bdev_integrity_enabled() is only used by bio_integrity_enabled().
Combine these two functions.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/block/data-integrity.txt | 10 -
block/bio-integrity.c | 39 +++---
include/linux/bio.h| 6 +++---
3 files changed, 20 insertions(+), 35 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index 2d735b0ae383..b4eacf48053c 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -192,16 +192,6 @@ will require extra work due to the application tag.
supported by the block device.
-int bdev_integrity_enabled(block_device, int rw);
-
- bdev_integrity_enabled() will return 1 if the block device
- supports integrity metadata transfer for the data direction
- specified in 'rw'.
-
- bdev_integrity_enabled() honors the write_generate and
- read_verify flags in sysfs and will respond accordingly.
-
-
int bio_integrity_prep(bio);
To generate IMD for WRITE and to set up buffers for READ, the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9e241063a616..6818c251e937 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -153,24 +153,6 @@ int bio_integrity_add_page(struct bio *bio, struct page
*page,
}
EXPORT_SYMBOL(bio_integrity_add_page);
-static int bdev_integrity_enabled(struct block_device *bdev, int rw)
-{
- struct blk_integrity *bi = bdev_get_integrity(bdev);
-
- if (bi == NULL)
- return 0;
-
- if (rw == READ bi-verify_fn != NULL
- (bi-flags INTEGRITY_FLAG_READ))
- return 1;
-
- if (rw == WRITE bi-generate_fn != NULL
- (bi-flags INTEGRITY_FLAG_WRITE))
- return 1;
-
- return 0;
-}
-
/**
* bio_integrity_enabled - Check whether integrity can be passed
* @bio: bio to check
@@ -180,16 +162,29 @@ static int bdev_integrity_enabled(struct block_device
*bdev, int rw)
* set prior to calling. The functions honors the write_generate and
* read_verify flags in sysfs.
*/
-int bio_integrity_enabled(struct bio *bio)
+bool bio_integrity_enabled(struct bio *bio)
{
+ struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
+
if (!bio_is_rw(bio))
- return 0;
+ return false;
/* Already protected? */
if (bio_integrity(bio))
- return 0;
+ return false;
+
+ if (bi == NULL)
+ return false;
+
+ if (bio_data_dir(bio) == READ bi-verify_fn != NULL
+ (bi-flags INTEGRITY_FLAG_READ))
+ return true;
+
+ if (bio_data_dir(bio) == WRITE bi-generate_fn != NULL
+ (bi-flags INTEGRITY_FLAG_WRITE))
+ return true;
- return bdev_integrity_enabled(bio-bi_bdev, bio_data_dir(bio));
+ return false;
}
EXPORT_SYMBOL(bio_integrity_enabled);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 5a645769f020..80ce1f732bb7 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -660,7 +660,7 @@ struct biovec_slab {
extern struct bio_integrity_payload *bio_integrity_alloc(struct bio *, gfp_t,
unsigned int);
extern void bio_integrity_free(struct bio *);
extern int bio_integrity_add_page(struct bio *, struct page *, unsigned int,
unsigned int);
-extern int bio_integrity_enabled(struct bio *bio);
+extern bool bio_integrity_enabled(struct bio *bio);
extern int bio_integrity_set_tag(struct bio *, void *, unsigned int);
extern int bio_integrity_get_tag(struct bio *, void *, unsigned int);
extern int bio_integrity_prep(struct bio *);
@@ -679,9 +679,9 @@ static inline int bio_integrity(struct bio *bio)
return 0;
}
-static inline int bio_integrity_enabled(struct bio *bio)
+static inline bool bio_integrity_enabled(struct bio *bio)
{
- return 0;
+ return false;
}
static inline int bioset_integrity_create(struct bio_set *bs, int pool_size)
--
1.9.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
[PATCH 03/14] block: Deprecate integrity tagging functions
None of the filesystems appear interested in using the integrity tagging
feature. Potentially because very few storage devices actually permit
using the application tag space.
Deprecate the tagging functions.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/block/data-integrity.txt | 34
block/bio-integrity.c | 94 +-
block/blk-integrity.c | 2 -
drivers/scsi/sd_dif.c | 65 ---
include/linux/bio.h| 3 --
include/linux/blkdev.h | 4 --
6 files changed, 1 insertion(+), 201 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index 4d4de8b09530..f56ec97f0d14 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -206,36 +206,6 @@ will require extra work due to the application tag.
bio_integrity_enabled() returned 1.
-int bio_integrity_tag_size(bio);
-
- If the filesystem wants to use the application tag space it will
- first have to find out how much storage space is available.
- Because tag space is generally limited (usually 2 bytes per
- sector regardless of sector size), the integrity framework
- supports interleaving the information between the sectors in an
- I/O.
-
- Filesystems can call bio_integrity_tag_size(bio) to find out how
- many bytes of storage are available for that particular bio.
-
- Another option is bdev_get_tag_size(block_device) which will
- return the number of available bytes per hardware sector.
-
-
-int bio_integrity_set_tag(bio, void *tag_buf, len);
-
- After a successful return from bio_integrity_prep(),
- bio_integrity_set_tag() can be used to attach an opaque tag
- buffer to a bio. Obviously this only makes sense if the I/O is
- a WRITE.
-
-
-int bio_integrity_get_tag(bio, void *tag_buf, len);
-
- Similarly, at READ I/O completion time the filesystem can
- retrieve the tag buffer using bio_integrity_get_tag().
-
-
5.3 PASSING EXISTING INTEGRITY METADATA
Filesystems that either generate their own integrity metadata or
@@ -288,8 +258,6 @@ will require extra work due to the application tag.
.name = STANDARDSBODY-TYPE-VARIANT-CSUM,
.generate_fn= my_generate_fn,
.verify_fn = my_verify_fn,
- .get_tag_fn = my_get_tag_fn,
- .set_tag_fn = my_set_tag_fn,
.tuple_size = sizeof(struct my_tuple_size),
.tag_size = tag bytes per hw sector,
};
@@ -311,7 +279,5 @@ will require extra work due to the application tag.
are available per hardware sector. For DIF this is either 2 or
0 depending on the value of the Control Mode Page ATO bit.
- See 6.2 for a description of get_tag_fn and set_tag_fn.
-
--
2007-12-24 Martin K. Petersen martin.peter...@oracle.com
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 25effd246deb..f59cdc2e0e63 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -216,90 +216,6 @@ static inline unsigned int bio_integrity_bytes(struct
blk_integrity *bi,
}
/**
- * bio_integrity_tag_size - Retrieve integrity tag space
- * @bio: bio to inspect
- *
- * Description: Returns the maximum number of tag bytes that can be
- * attached to this bio. Filesystems can use this to determine how
- * much metadata to attach to an I/O.
- */
-unsigned int bio_integrity_tag_size(struct bio *bio)
-{
- struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
-
- BUG_ON(bio-bi_iter.bi_size == 0);
-
- return bi-tag_size * (bio-bi_iter.bi_size / bi-sector_size);
-}
-EXPORT_SYMBOL(bio_integrity_tag_size);
-
-static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
-int set)
-{
- struct bio_integrity_payload *bip = bio_integrity(bio);
- struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
- unsigned int nr_sectors;
-
- BUG_ON(bip-bip_buf == NULL);
-
- if (bi-tag_size == 0)
- return -1;
-
- nr_sectors = bio_integrity_hw_sectors(bi,
- DIV_ROUND_UP(len, bi-tag_size));
-
- if (nr_sectors * bi-tuple_size bip-bip_iter.bi_size) {
- printk(KERN_ERR %s: tag too big for bio: %u %u\n, __func__,
- nr_sectors * bi-tuple_size, bip-bip_iter.bi_size);
- return -1;
- }
-
- if (set)
- bi-set_tag_fn(bip-bip_buf, tag_buf, nr_sectors);
- else
- bi-get_tag_fn(bip-bip_buf, tag_buf, nr_sectors);
-
- return 0;
-}
-
-/**
- *
[PATCH 02/14] block: Replace bi_integrity with bi_special
For commands like REQ_COPY we need a way to pass extra information along
with each bio. Like integrity metadata this information must be
available at the bottom of the stack so bi_private does not suffice.
Rename the existing bi_integrity field to bi_special and make it a union
so we can have different bio extensions for each class of command.
We previously used bi_integrity != NULL as a way to identify whether a
bio had integrity metadata or not. Introduce a REQ_INTEGRITY to be the
indicator now that bi_special can contain different things.
In addition, bio_integrity(bio) will now return a pointer to the
integrity payload (when applicable).
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/block/data-integrity.txt | 10 +-
block/bio-integrity.c | 23 ---
drivers/scsi/sd_dif.c | 8
include/linux/bio.h| 10 +++---
include/linux/blk_types.h | 8 ++--
include/linux/blkdev.h | 7 ++-
6 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index b4eacf48053c..4d4de8b09530 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -129,11 +129,11 @@ interface for this is being worked on.
4.1 BIO
The data integrity patches add a new field to struct bio when
-CONFIG_BLK_DEV_INTEGRITY is enabled. bio-bi_integrity is a pointer
-to a struct bip which contains the bio integrity payload. Essentially
-a bip is a trimmed down struct bio which holds a bio_vec containing
-the integrity metadata and the required housekeeping information (bvec
-pool, vector count, etc.)
+CONFIG_BLK_DEV_INTEGRITY is enabled. bio_integrity(bio) returns a
+pointer to a struct bip which contains the bio integrity payload.
+Essentially a bip is a trimmed down struct bio which holds a bio_vec
+containing the integrity metadata and the required housekeeping
+information (bvec pool, vector count, etc.)
A kernel subsystem can enable data integrity protection on a bio by
calling bio_integrity_alloc(bio). This will allocate and attach the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 6818c251e937..25effd246deb 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -76,7 +76,8 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio
*bio,
bip-bip_slab = idx;
bip-bip_bio = bio;
- bio-bi_integrity = bip;
+ bio-bi_special.integrity = bip;
+ bio-bi_rw |= REQ_INTEGRITY;
return bip;
err:
@@ -94,7 +95,7 @@ EXPORT_SYMBOL(bio_integrity_alloc);
*/
void bio_integrity_free(struct bio *bio)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_set *bs = bio-bi_pool;
if (bip-bip_owns_buf)
@@ -110,7 +111,7 @@ void bio_integrity_free(struct bio *bio)
kfree(bip);
}
- bio-bi_integrity = NULL;
+ bio-bi_special.integrity = NULL;
}
EXPORT_SYMBOL(bio_integrity_free);
@@ -134,7 +135,7 @@ static inline unsigned int bip_integrity_vecs(struct
bio_integrity_payload *bip)
int bio_integrity_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_vec *iv;
if (bip-bip_vcnt = bip_integrity_vecs(bip)) {
@@ -235,7 +236,7 @@ EXPORT_SYMBOL(bio_integrity_tag_size);
static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
int set)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
unsigned int nr_sectors;
@@ -310,12 +311,12 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct bio_vec *bv;
sector_t sector;
unsigned int sectors, ret = 0, i;
- void *prot_buf = bio-bi_integrity-bip_buf;
+ void *prot_buf = bio_integrity(bio)-bip_buf;
if (operate)
sector = bio-bi_iter.bi_sector;
else
- sector = bio-bi_integrity-bip_iter.bi_sector;
+ sector = bio_integrity(bio)-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
bix.sector_size = bi-sector_size;
@@ -511,7 +512,7 @@ static void bio_integrity_verify_fn(struct work_struct
*work)
*/
void bio_integrity_endio(struct bio *bio, int error)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
BUG_ON(bip-bip_bio != bio);
@@ -542,7 +543,7 @@
[PATCH 13/14] lib: Add T10 Protection Information functions
The T10 Protection Information format is also used by some devices that
do not go through the SCSI layer (virtual block devices, NVMe). Relocate
the relevant functions to a library that can be used without involving
SCSI.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/Kconfig | 1 +
drivers/scsi/Kconfig | 2 +-
drivers/scsi/sd_dif.c | 193 +++--
include/linux/crc-t10dif.h | 5 +-
include/linux/t10-pi.h | 28 +++
lib/Kconfig| 7 ++
lib/Makefile | 2 +
lib/t10-pi.c | 164 ++
8 files changed, 219 insertions(+), 183 deletions(-)
create mode 100644 include/linux/t10-pi.h
create mode 100644 lib/t10-pi.c
diff --git a/block/Kconfig b/block/Kconfig
index 2429515c05c2..947658db8456 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -77,6 +77,7 @@ config BLK_DEV_BSGLIB
config BLK_DEV_INTEGRITY
bool Block layer data integrity support
+ select T10_PI if BLK_DEV_INTEGRITY
---help---
Some storage devices allow extra information to be
stored/retrieved to help protect the data. The block layer
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 02832d64d918..1096b16b2a0c 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -69,7 +69,7 @@ comment SCSI support type (disk, tape, CD-ROM)
config BLK_DEV_SD
tristate SCSI disk support
depends on SCSI
- select CRC_T10DIF if BLK_DEV_INTEGRITY
+ select T10_PI if BLK_DEV_INTEGRITY
---help---
If you want to use SCSI hard disks, Fibre Channel disks,
Serial ATA (SATA) or Parallel ATA (PATA) hard disks,
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 033d30d37952..d7109fff327d 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -21,7 +21,7 @@
*/
#include linux/blkdev.h
-#include linux/crc-t10dif.h
+#include linux/t10-pi.h
#include scsi/scsi.h
#include scsi/scsi_cmnd.h
@@ -33,204 +33,37 @@
#include scsi/scsi_ioctl.h
#include scsi/scsicam.h
-#include net/checksum.h
-
#include sd.h
-typedef __u16 (csum_fn) (void *, unsigned int);
-
-static __u16 sd_dif_crc_fn(void *data, unsigned int len)
-{
- return cpu_to_be16(crc_t10dif(data, len));
-}
-
-static __u16 sd_dif_ip_fn(void *data, unsigned int len)
-{
- return ip_compute_csum(data, len);
-}
-
-/*
- * Type 1 and Type 2 protection use the same format: 16 bit guard tag,
- * 16 bit app tag, 32 bit reference tag.
- */
-static void sd_dif_type1_generate(struct blk_integrity_iter *iter, csum_fn *fn)
-{
- void *buf = iter-data_buf;
- struct sd_dif_tuple *sdt = iter-prot_buf;
- sector_t seed = iter-seed;
- unsigned int i;
-
- for (i = 0 ; i iter-data_size ; i += iter-interval, sdt++) {
- sdt-guard_tag = fn(buf, iter-interval);
- sdt-ref_tag = cpu_to_be32(seed 0x);
- sdt-app_tag = 0;
-
- buf += iter-interval;
- seed++;
- }
-}
-
-static int sd_dif_type1_generate_crc(struct blk_integrity_iter *iter)
-{
- sd_dif_type1_generate(iter, sd_dif_crc_fn);
- return 0;
-}
-
-static int sd_dif_type1_generate_ip(struct blk_integrity_iter *iter)
-{
- sd_dif_type1_generate(iter, sd_dif_ip_fn);
- return 0;
-}
-
-static int sd_dif_type1_verify(struct blk_integrity_iter *iter, csum_fn *fn)
-{
- void *buf = iter-data_buf;
- struct sd_dif_tuple *sdt = iter-prot_buf;
- sector_t seed = iter-seed;
- unsigned int i;
- __u16 csum;
-
- for (i = 0 ; i iter-data_size ; i += iter-interval, sdt++) {
- /* Unwritten sectors */
- if (sdt-app_tag == 0x)
- return 0;
-
- if (be32_to_cpu(sdt-ref_tag) != (seed 0x)) {
- printk(KERN_ERR
- %s: ref tag error on sector %lu (rcvd %u)\n,
- iter-disk_name, (unsigned long)seed,
- be32_to_cpu(sdt-ref_tag));
- return -EIO;
- }
-
- csum = fn(buf, iter-interval);
-
- if (sdt-guard_tag != csum) {
- printk(KERN_ERR %s: guard tag error on sector %lu \
- (rcvd %04x, data %04x)\n, iter-disk_name,
- (unsigned long)seed,
- be16_to_cpu(sdt-guard_tag), be16_to_cpu(csum));
- return -EIO;
- }
-
- buf += iter-interval;
- seed++;
- }
-
- return 0;
-}
-
-static int sd_dif_type1_verify_crc(struct blk_integrity_iter *iter)
-{
- return sd_dif_type1_verify(iter, sd_dif_crc_fn);
-}
-
-static int sd_dif_type1_verify_ip(struct blk_integrity_iter *iter)
-{
- return sd_dif_type1_verify(iter,
[PATCH 05/14] block: Deprecate the use of the term sector in the context of block integrity
The protection interval is not necessarily tied to the logical block
size of a block device. Stop using the terms sector and sectors.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/bio-integrity.c | 46 +-
block/blk-integrity.c | 10 +-
drivers/scsi/sd_dif.c | 46 +++---
include/linux/blkdev.h | 6 +++---
4 files changed, 52 insertions(+), 56 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e06b3c807eef..c52a8fd98706 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -191,29 +191,25 @@ bool bio_integrity_enabled(struct bio *bio)
EXPORT_SYMBOL(bio_integrity_enabled);
/**
- * bio_integrity_hw_sectors - Convert 512b sectors to hardware ditto
+ * bio_integrity_intervals - Return number of integrity intervals for a bio
* @bi:blk_integrity profile for device
- * @sectors: Number of 512 sectors to convert
+ * @sectors: Size of the bio in 512-byte sectors
*
* Description: The block layer calculates everything in 512 byte
- * sectors but integrity metadata is done in terms of the hardware
- * sector size of the storage device. Convert the block layer sectors
- * to physical sectors.
+ * sectors but integrity metadata is done in terms of the data integrity
+ * interval size of the storage device. Convert the block layer sectors
+ * to the appropriate number of integrity intervals.
*/
-static inline unsigned int bio_integrity_hw_sectors(struct blk_integrity *bi,
- unsigned int sectors)
+static inline unsigned int bio_integrity_intervals(struct blk_integrity *bi,
+ unsigned int sectors)
{
- /* At this point there are only 512b or 4096b DIF/EPP devices */
- if (bi-sector_size == 4096)
- return sectors = 3;
-
- return sectors;
+ return sectors (ilog2(bi-interval) - 9);
}
static inline unsigned int bio_integrity_bytes(struct blk_integrity *bi,
unsigned int sectors)
{
- return bio_integrity_hw_sectors(bi, sectors) * bi-tuple_size;
+ return bio_integrity_intervals(bi, sectors) * bi-tuple_size;
}
/**
@@ -227,25 +223,25 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct blk_integrity_exchg bix;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
- sector_t sector;
- unsigned int sectors, ret = 0, i;
+ sector_t seed;
+ unsigned int intervals, ret = 0, i;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
if (operate)
- sector = bio-bi_iter.bi_sector;
+ seed = bio-bi_iter.bi_sector;
else
- sector = bip-bip_iter.bi_sector;
+ seed = bip-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
- bix.sector_size = bi-sector_size;
+ bix.interval = bi-interval;
bio_for_each_segment_all(bv, bio, i) {
void *kaddr = kmap_atomic(bv-bv_page);
bix.data_buf = kaddr + bv-bv_offset;
bix.data_size = bv-bv_len;
bix.prot_buf = prot_buf;
- bix.sector = sector;
+ bix.seed = seed;
if (operate)
bi-generate_fn(bix);
@@ -257,9 +253,9 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
}
}
- sectors = bv-bv_len / bi-sector_size;
- sector += sectors;
- prot_buf += sectors * bi-tuple_size;
+ intervals = bv-bv_len / bi-interval;
+ seed += intervals;
+ prot_buf += intervals * bi-tuple_size;
kunmap_atomic(kaddr);
}
@@ -300,17 +296,17 @@ int bio_integrity_prep(struct bio *bio)
unsigned long start, end;
unsigned int len, nr_pages;
unsigned int bytes, offset, i;
- unsigned int sectors;
+ unsigned int intervals;
bi = bdev_get_integrity(bio-bi_bdev);
q = bdev_get_queue(bio-bi_bdev);
BUG_ON(bi == NULL);
BUG_ON(bio_integrity(bio));
- sectors = bio_integrity_hw_sectors(bi, bio_sectors(bio));
+ intervals = bio_integrity_intervals(bi, bio_sectors(bio));
/* Allocate kernel buffer for protection data */
- len = sectors * bi-tuple_size;
+ len = intervals * bi-tuple_size;
buf = kmalloc(len, GFP_NOIO | q-bounce_gfp);
if (unlikely(buf == NULL)) {
printk(KERN_ERR could not allocate integrity buffer\n);
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 7ac17160ab69..3760d0aeed92 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -154,10
[PATCH 11/14] block: Don't merge requests if integrity flags differ
We'd occasionally merge requests with conflicting integrity flags.
Introduce a merge helper which checks that the requests have compatible
integrity payloads.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/blk-integrity.c | 36 ++--
block/blk-merge.c | 6 +++---
include/linux/blkdev.h | 20 ++--
3 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 8cf87655152b..da608e73bdb1 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -186,37 +186,53 @@ int blk_integrity_compare(struct gendisk *gd1, struct
gendisk *gd2)
}
EXPORT_SYMBOL(blk_integrity_compare);
-int blk_integrity_merge_rq(struct request_queue *q, struct request *req,
- struct request *next)
+bool blk_integrity_merge_rq(struct request_queue *q, struct request *req,
+ struct request *next)
{
- if (blk_integrity_rq(req) != blk_integrity_rq(next))
- return -1;
+ if (blk_integrity_rq(req) == 0 blk_integrity_rq(next) == 0)
+ return true;
+
+ if (blk_integrity_rq(req) == 0 || blk_integrity_rq(next) == 0)
+ return false;
+
+ if (bio_integrity(req-bio)-bip_flags !=
+ bio_integrity(next-bio)-bip_flags)
+ return false;
if (req-nr_integrity_segments + next-nr_integrity_segments
q-limits.max_integrity_segments)
- return -1;
+ return false;
- return 0;
+ return true;
}
EXPORT_SYMBOL(blk_integrity_merge_rq);
-int blk_integrity_merge_bio(struct request_queue *q, struct request *req,
- struct bio *bio)
+bool blk_integrity_merge_bio(struct request_queue *q, struct request *req,
+struct bio *bio)
{
int nr_integrity_segs;
struct bio *next = bio-bi_next;
+ if (blk_integrity_rq(req) == 0 bio_integrity(bio) == NULL)
+ return true;
+
+ if (blk_integrity_rq(req) == 0 || bio_integrity(bio) == NULL)
+ return false;
+
+ if (bio_integrity(req-bio)-bip_flags != bio_integrity(bio)-bip_flags)
+ return false;
+
bio-bi_next = NULL;
nr_integrity_segs = blk_rq_count_integrity_sg(q, bio);
bio-bi_next = next;
if (req-nr_integrity_segments + nr_integrity_segs
q-limits.max_integrity_segments)
- return -1;
+ return false;
req-nr_integrity_segments += nr_integrity_segs;
- return 0;
+ return true;
}
EXPORT_SYMBOL(blk_integrity_merge_bio);
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 6c583f9c5b65..21e38f407785 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -294,7 +294,7 @@ static inline int ll_new_hw_segment(struct request_queue *q,
if (req-nr_phys_segments + nr_phys_segs queue_max_segments(q))
goto no_merge;
- if (bio_integrity(bio) blk_integrity_merge_bio(q, req, bio))
+ if (blk_integrity_merge_bio(q, req, bio) == false)
goto no_merge;
/*
@@ -391,7 +391,7 @@ static int ll_merge_requests_fn(struct request_queue *q,
struct request *req,
if (total_phys_segments queue_max_segments(q))
return 0;
- if (blk_integrity_rq(req) blk_integrity_merge_rq(q, req, next))
+ if (blk_integrity_merge_rq(q, req, next) == false)
return 0;
/* Merge is OK... */
@@ -569,7 +569,7 @@ bool blk_rq_merge_ok(struct request *rq, struct bio *bio)
return false;
/* only merge integrity protected bio into ditto rq */
- if (bio_integrity(bio) != blk_integrity_rq(rq))
+ if (blk_integrity_merge_bio(rq-q, rq, bio) == false)
return false;
/* must be using the same buffer */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9bf6f761f1ac..45cd70cda4e8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1467,10 +1467,10 @@ extern int blk_integrity_compare(struct gendisk *,
struct gendisk *);
extern int blk_rq_map_integrity_sg(struct request_queue *, struct bio *,
struct scatterlist *);
extern int blk_rq_count_integrity_sg(struct request_queue *, struct bio *);
-extern int blk_integrity_merge_rq(struct request_queue *, struct request *,
- struct request *);
-extern int blk_integrity_merge_bio(struct request_queue *, struct request *,
- struct bio *);
+extern bool blk_integrity_merge_rq(struct request_queue *, struct request *,
+ struct request *);
+extern bool blk_integrity_merge_bio(struct request_queue *, struct request *,
+ struct bio *);
static inline
struct blk_integrity *bdev_get_integrity(struct block_device
[PATCH 12/14] block: Add specific data integrity errors
Introduce a set of error codes that can be used by the block integrity
subsystem to signal which class of error was encountered by either the
I/O controller or the storage device.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/blk-core.c | 12
drivers/md/dm-mpath.c| 9 +
drivers/scsi/scsi_lib.c | 30 --
include/uapi/asm-generic/errno.h | 11 +++
4 files changed, 60 insertions(+), 2 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5b6f768a7c01..9e8b9649baf7 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2428,6 +2428,18 @@ bool blk_update_request(struct request *req, int error,
unsigned int nr_bytes)
case -ENODATA:
error_type = critical medium;
break;
+ case -ECTRLGRD:
+ case -ECTRLAPP:
+ case -ECTRLREF:
+ case -EDISKGRD:
+ case -EDISKAPP:
+ case -EDISKREF:
+ case -EKERNGRD:
+ case -EKERNAPP:
+ case -EKERNREF:
+ case -EILSEQ:
+ error_type = data integrity;
+ break;
case -EIO:
default:
error_type = I/O;
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index aa009e865871..a8756cdd1002 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1208,6 +1208,15 @@ static int noretry_error(int error)
case -EOPNOTSUPP:
case -EREMOTEIO:
case -EILSEQ:
+ case -ECTRLGRD:
+ case -ECTRLAPP:
+ case -ECTRLREF:
+ case -EDISKGRD:
+ case -EDISKAPP:
+ case -EDISKREF:
+ case -EKERNGRD:
+ case -EKERNAPP:
+ case -EKERNREF:
case -ENODATA:
case -ENOSPC:
return 1;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 3cc82d3dec78..4f369d6f1162 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -892,7 +892,20 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
} else if (sshdr.asc == 0x10) /* DIX */ {
description = Host Data Integrity Failure;
action = ACTION_FAIL;
- error = -EILSEQ;
+ switch (sshdr.ascq) {
+ case 0x1:
+ error = -ECTRLGRD;
+ break;
+ case 0x2:
+ error = -ECTRLAPP;
+ break;
+ case 0x3:
+ error = -ECTRLREF;
+ break;
+ default:
+ error = -EILSEQ;
+ break;
+ }
/* INVALID COMMAND OPCODE or INVALID FIELD IN CDB */
} else if (sshdr.asc == 0x20 || sshdr.asc == 0x24) {
switch (cmd-cmnd[0]) {
@@ -920,7 +933,20 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned
int good_bytes)
action = ACTION_FAIL;
if (sshdr.asc == 0x10) { /* DIF */
description = Target Data Integrity Failure;
- error = -EILSEQ;
+ switch (sshdr.ascq) {
+ case 0x1:
+ error = -EDISKGRD;
+ break;
+ case 0x2:
+ error = -EDISKAPP;
+ break;
+ case 0x3:
+ error = -EDISKREF;
+ break;
+ default:
+ error = -EILSEQ;
+ break;
+ }
}
break;
case NOT_READY:
diff --git a/include/uapi/asm-generic/errno.h b/include/uapi/asm-generic/errno.h
index 1e1ea6e6e7a5..d89af4ba1e04 100644
--- a/include/uapi/asm-generic/errno.h
+++ b/include/uapi/asm-generic/errno.h
@@ -110,4 +110,15 @@
#define EHWPOISON 133 /* Memory page has hardware error */
+/* data integrity errors */
+#define ECTRLGRD 134 /* I/O controller detected guard tag error */
+#define ECTRLAPP 135 /* I/O controller detected app tag error */
+#define ECTRLREF 136 /* I/O controller detected ref tag error */
+#define EDISKGRD
[PATCH 07/14] block: Add prefix to block integrity profile flags
Add a BLK_ prefix to the integrity profile flags. Also rename the flags
to be more consistent with the generate/verify terminology in the rest
of the integrity code.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/bio-integrity.c | 4 ++--
block/blk-integrity.c | 43 ++-
include/linux/blkdev.h | 6 --
3 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index e711b9c71767..c91181e3d18d 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -179,11 +179,11 @@ bool bio_integrity_enabled(struct bio *bio)
return false;
if (bio_data_dir(bio) == READ bi-verify_fn != NULL
- (bi-flags INTEGRITY_FLAG_READ))
+ (bi-flags BLK_INTEGRITY_VERIFY))
return true;
if (bio_data_dir(bio) == WRITE bi-generate_fn != NULL
- (bi-flags INTEGRITY_FLAG_WRITE))
+ (bi-flags BLK_INTEGRITY_GENERATE))
return true;
return false;
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 3760d0aeed92..95f451a3c581 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -269,42 +269,42 @@ static ssize_t integrity_tag_size_show(struct
blk_integrity *bi, char *page)
return sprintf(page, 0\n);
}
-static ssize_t integrity_read_store(struct blk_integrity *bi,
- const char *page, size_t count)
+static ssize_t integrity_verify_store(struct blk_integrity *bi,
+ const char *page, size_t count)
{
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
if (val)
- bi-flags |= INTEGRITY_FLAG_READ;
+ bi-flags |= BLK_INTEGRITY_VERIFY;
else
- bi-flags = ~INTEGRITY_FLAG_READ;
+ bi-flags = ~BLK_INTEGRITY_VERIFY;
return count;
}
-static ssize_t integrity_read_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_verify_show(struct blk_integrity *bi, char *page)
{
- return sprintf(page, %d\n, (bi-flags INTEGRITY_FLAG_READ) != 0);
+ return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_VERIFY) != 0);
}
-static ssize_t integrity_write_store(struct blk_integrity *bi,
-const char *page, size_t count)
+static ssize_t integrity_generate_store(struct blk_integrity *bi,
+ const char *page, size_t count)
{
char *p = (char *) page;
unsigned long val = simple_strtoul(p, p, 10);
if (val)
- bi-flags |= INTEGRITY_FLAG_WRITE;
+ bi-flags |= BLK_INTEGRITY_GENERATE;
else
- bi-flags = ~INTEGRITY_FLAG_WRITE;
+ bi-flags = ~BLK_INTEGRITY_GENERATE;
return count;
}
-static ssize_t integrity_write_show(struct blk_integrity *bi, char *page)
+static ssize_t integrity_generate_show(struct blk_integrity *bi, char *page)
{
- return sprintf(page, %d\n, (bi-flags INTEGRITY_FLAG_WRITE) != 0);
+ return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_GENERATE) != 0);
}
static struct integrity_sysfs_entry integrity_format_entry = {
@@ -317,23 +317,23 @@ static struct integrity_sysfs_entry
integrity_tag_size_entry = {
.show = integrity_tag_size_show,
};
-static struct integrity_sysfs_entry integrity_read_entry = {
+static struct integrity_sysfs_entry integrity_verify_entry = {
.attr = { .name = read_verify, .mode = S_IRUGO | S_IWUSR },
- .show = integrity_read_show,
- .store = integrity_read_store,
+ .show = integrity_verify_show,
+ .store = integrity_verify_store,
};
-static struct integrity_sysfs_entry integrity_write_entry = {
+static struct integrity_sysfs_entry integrity_generate_entry = {
.attr = { .name = write_generate, .mode = S_IRUGO | S_IWUSR },
- .show = integrity_write_show,
- .store = integrity_write_store,
+ .show = integrity_generate_show,
+ .store = integrity_generate_store,
};
static struct attribute *integrity_attrs[] = {
integrity_format_entry.attr,
integrity_tag_size_entry.attr,
- integrity_read_entry.attr,
- integrity_write_entry.attr,
+ integrity_verify_entry.attr,
+ integrity_generate_entry.attr,
NULL,
};
@@ -406,7 +406,7 @@ int blk_integrity_register(struct gendisk *disk, struct
blk_integrity *template)
kobject_uevent(bi-kobj, KOBJ_ADD);
- bi-flags |= INTEGRITY_FLAG_READ | INTEGRITY_FLAG_WRITE;
+ bi-flags |= BLK_INTEGRITY_VERIFY | BLK_INTEGRITY_GENERATE;
bi-interval = queue_logical_block_size(disk-queue);
disk-integrity = bi;
} else
@@ -419,6 +419,7 @@ int blk_integrity_register(struct gendisk *disk, struct
blk_integrity *template)
[PATCH 04/14] block: Remove bip_buf
bip_buf is not really needed so we can remove it.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/bio-integrity.c | 10 ++
include/linux/bio.h | 3 ---
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index f59cdc2e0e63..e06b3c807eef 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -99,7 +99,8 @@ void bio_integrity_free(struct bio *bio)
struct bio_set *bs = bio-bi_pool;
if (bip-bip_owns_buf)
- kfree(bip-bip_buf);
+ kfree(page_address(bip-bip_vec-bv_page) +
+ bip-bip_vec-bv_offset);
if (bs) {
if (bip-bip_slab != BIO_POOL_NONE)
@@ -225,14 +226,16 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
struct blk_integrity_exchg bix;
struct bio_vec *bv;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
sector_t sector;
unsigned int sectors, ret = 0, i;
- void *prot_buf = bio_integrity(bio)-bip_buf;
+ void *prot_buf = page_address(bip-bip_vec-bv_page) +
+ bip-bip_vec-bv_offset;
if (operate)
sector = bio-bi_iter.bi_sector;
else
- sector = bio_integrity(bio)-bip_iter.bi_sector;
+ sector = bip-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
bix.sector_size = bi-sector_size;
@@ -327,7 +330,6 @@ int bio_integrity_prep(struct bio *bio)
}
bip-bip_owns_buf = 1;
- bip-bip_buf = buf;
bip-bip_iter.bi_size = len;
bip-bip_iter.bi_sector = bio-bi_iter.bi_sector;
diff --git a/include/linux/bio.h b/include/linux/bio.h
index f7fa470c043e..5d9c7680280b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -292,9 +292,6 @@ struct bio_integrity_payload {
struct bvec_iterbip_iter;
- /* kill - should just use bip_vec */
- void*bip_buf; /* generated integrity data */
-
bio_end_io_t*bip_end_io;/* saved I/O completion fn */
unsigned short bip_slab; /* slab the bip came from */
--
1.9.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
[PATCH 06/14] block: Clean up the code used to generate and verify integrity metadata
Instead of the operate parameter we pass in a seed value and a pointer
to a function that can be used to process the integrity metadata. The
generation function is changed to have a return value to fit into this
scheme.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/bio-integrity.c | 82 ++
drivers/scsi/sd_dif.c | 106 ++---
include/linux/bio.h| 12 ++
include/linux/blkdev.h | 9 ++---
4 files changed, 94 insertions(+), 115 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index c52a8fd98706..e711b9c71767 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -213,49 +213,37 @@ static inline unsigned int bio_integrity_bytes(struct
blk_integrity *bi,
}
/**
- * bio_integrity_generate_verify - Generate/verify integrity metadata for a bio
+ * bio_integrity_process - Process integrity metadata for a bio
* @bio: bio to generate/verify integrity metadata for
- * @operate: operate number, 1 for generate, 0 for verify
+ * @proc_fn: Pointer to the relevant processing function
*/
-static int bio_integrity_generate_verify(struct bio *bio, int operate)
+static int bio_integrity_process(struct bio *bio,
+integrity_processing_fn *proc_fn)
{
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
- struct blk_integrity_exchg bix;
+ struct blk_integrity_iter iter;
struct bio_vec *bv;
struct bio_integrity_payload *bip = bio_integrity(bio);
- sector_t seed;
- unsigned int intervals, ret = 0, i;
+ unsigned int i, ret = 0;
void *prot_buf = page_address(bip-bip_vec-bv_page) +
bip-bip_vec-bv_offset;
- if (operate)
- seed = bio-bi_iter.bi_sector;
- else
- seed = bip-bip_iter.bi_sector;
-
- bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
- bix.interval = bi-interval;
+ iter.disk_name = bio-bi_bdev-bd_disk-disk_name;
+ iter.interval = bi-interval;
+ iter.seed = bip_get_seed(bip);
+ iter.prot_buf = prot_buf;
bio_for_each_segment_all(bv, bio, i) {
void *kaddr = kmap_atomic(bv-bv_page);
- bix.data_buf = kaddr + bv-bv_offset;
- bix.data_size = bv-bv_len;
- bix.prot_buf = prot_buf;
- bix.seed = seed;
-
- if (operate)
- bi-generate_fn(bix);
- else {
- ret = bi-verify_fn(bix);
- if (ret) {
- kunmap_atomic(kaddr);
- return ret;
- }
- }
- intervals = bv-bv_len / bi-interval;
- seed += intervals;
- prot_buf += intervals * bi-tuple_size;
+ iter.data_buf = kaddr + bv-bv_offset;
+ iter.data_size = bv-bv_len;
+
+ ret = proc_fn(iter);
+ if (ret) {
+ kunmap_atomic(kaddr);
+ return ret;
+ }
kunmap_atomic(kaddr);
}
@@ -263,20 +251,6 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
}
/**
- * bio_integrity_generate - Generate integrity metadata for a bio
- * @bio: bio to generate integrity metadata for
- *
- * Description: Generates integrity metadata for a bio by calling the
- * block device's generation callback function. The bio must have a
- * bip attached with enough room to accommodate the generated
- * integrity metadata.
- */
-static void bio_integrity_generate(struct bio *bio)
-{
- bio_integrity_generate_verify(bio, 1);
-}
-
-/**
* bio_integrity_prep - Prepare bio for integrity I/O
* @bio: bio to prepare
*
@@ -327,7 +301,7 @@ int bio_integrity_prep(struct bio *bio)
bip-bip_owns_buf = 1;
bip-bip_iter.bi_size = len;
- bip-bip_iter.bi_sector = bio-bi_iter.bi_sector;
+ bip_set_seed(bip, bio-bi_iter.bi_sector);
/* Map it */
offset = offset_in_page(buf);
@@ -363,26 +337,13 @@ int bio_integrity_prep(struct bio *bio)
/* Auto-generate integrity metadata if this is a write */
if (bio_data_dir(bio) == WRITE)
- bio_integrity_generate(bio);
+ bio_integrity_process(bio, bi-generate_fn);
return 0;
}
EXPORT_SYMBOL(bio_integrity_prep);
/**
- * bio_integrity_verify - Verify integrity metadata for a bio
- * @bio: bio to verify
- *
- * Description: This function is called to verify the integrity of a
- * bio. The data in the bio io_vec is compared to the integrity
- * metadata returned by the HBA.
- */
-static int bio_integrity_verify(struct bio *bio)
-{
- return bio_integrity_generate_verify(bio, 0);
-}
-
-/**
* bio_integrity_verify_fn - Integrity I/O completion worker
*
[PATCH 08/14] block: Add a disk flag to block integrity profile
So far we have relied on the app tag size to determine whether a disk
has been formatted with T10 protection information or not. However, not
all target devices provide application tag storage.
Add a flag to the block integrity profile that indicates whether the
disk has been formatted with protection information.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/ABI/testing/sysfs-block | 9 +
block/blk-integrity.c | 11 +++
drivers/scsi/sd_dif.c | 8 +++-
include/linux/blkdev.h| 1 +
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/sysfs-block
b/Documentation/ABI/testing/sysfs-block
index 279da08f7541..5876e163c430 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -53,6 +53,15 @@ Description:
512 bytes of data.
+What: /sys/block/disk/integrity/disk
+Date: February 2011
+Contact: Martin K. Petersen martin.peter...@oracle.com
+Description:
+ Indicates whether a storage device is capable of
+ persistently storing integrity metadata. Set if the
+ device is T10 PI-capable.
+
+
What: /sys/block/disk/integrity/write_generate
Date: June 2008
Contact: Martin K. Petersen martin.peter...@oracle.com
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 95f451a3c581..8cf87655152b 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -307,6 +307,11 @@ static ssize_t integrity_generate_show(struct
blk_integrity *bi, char *page)
return sprintf(page, %d\n, (bi-flags BLK_INTEGRITY_GENERATE) != 0);
}
+static ssize_t integrity_disk_show(struct blk_integrity *bi, char *page)
+{
+ return sprintf(page, %u\n, (bi-flags BLK_INTEGRITY_DISK) != 0);
+}
+
static struct integrity_sysfs_entry integrity_format_entry = {
.attr = { .name = format, .mode = S_IRUGO },
.show = integrity_format_show,
@@ -329,11 +334,17 @@ static struct integrity_sysfs_entry
integrity_generate_entry = {
.store = integrity_generate_store,
};
+static struct integrity_sysfs_entry integrity_disk_entry = {
+ .attr = { .name = disk, .mode = S_IRUGO },
+ .show = integrity_disk_show,
+};
+
static struct attribute *integrity_attrs[] = {
integrity_format_entry.attr,
integrity_tag_size_entry.attr,
integrity_verify_entry.attr,
integrity_generate_entry.attr,
+ integrity_disk_entry.attr,
NULL,
};
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 801c41851a01..1d401f864fbe 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -270,7 +270,13 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
Enabling DIX %s protection\n, disk-integrity-name);
/* Signal to block layer that we support sector tagging */
- if (dif type sdkp-ATO) {
+ if (dif type) {
+
+ disk-integrity-flags |= BLK_INTEGRITY_DISK;
+
+ if (!sdkp)
+ return;
+
if (type == SD_DIF_TYPE3_PROTECTION)
disk-integrity-tag_size = sizeof(u16) + sizeof(u32);
else
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index bb44630d27f8..4be0446a8817 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1431,6 +1431,7 @@ static inline uint64_t rq_io_start_time_ns(struct request
*req)
enum blk_integrity_flags {
BLK_INTEGRITY_VERIFY= 1 0,
BLK_INTEGRITY_GENERATE = 1 1,
+ BLK_INTEGRITY_DISK = 1 2,
};
struct blk_integrity_iter {
--
1.9.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
[PATCH 10/14] block: Integrity checksum flag
Make the choice of checksum a per-I/O property by introducing a flag
that can be inspected by the SCSI layer.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/bio-integrity.c | 3 +++
drivers/scsi/sd_dif.c | 6 --
include/linux/bio.h| 1 +
include/linux/blkdev.h | 1 +
4 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 877bce028766..4eb7893a7559 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -303,6 +303,9 @@ int bio_integrity_prep(struct bio *bio)
bip-bip_iter.bi_size = len;
bip_set_seed(bip, bio-bi_iter.bi_sector);
+ if (bi-flags BLK_INTEGRITY_IP_CHECKSUM)
+ bip_set_flag(bip, BIP_IP_CHECKSUM);
+
/* Map it */
offset = offset_in_page(buf);
for (i = 0 ; i nr_pages ; i++) {
diff --git a/drivers/scsi/sd_dif.c b/drivers/scsi/sd_dif.c
index 95d5cb806f58..033d30d37952 100644
--- a/drivers/scsi/sd_dif.c
+++ b/drivers/scsi/sd_dif.c
@@ -255,12 +255,14 @@ void sd_dif_config_host(struct scsi_disk *sdkp)
return;
/* Enable DMA of protection information */
- if (scsi_host_get_guard(sdkp-device-host) SHOST_DIX_GUARD_IP)
+ if (scsi_host_get_guard(sdkp-device-host) SHOST_DIX_GUARD_IP) {
if (type == SD_DIF_TYPE3_PROTECTION)
blk_integrity_register(disk, dif_type3_integrity_ip);
else
blk_integrity_register(disk, dif_type1_integrity_ip);
- else
+
+ disk-integrity-flags |= BLK_INTEGRITY_IP_CHECKSUM;
+ } else
if (type == SD_DIF_TYPE3_PROTECTION)
blk_integrity_register(disk, dif_type3_integrity_crc);
else
diff --git a/include/linux/bio.h b/include/linux/bio.h
index adc806325c36..83e4725f6aca 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -309,6 +309,7 @@ enum bip_flags {
BIP_MAPPED_INTEGRITY, /* integrity metadata has been remapped */
BIP_CTRL_NOCHECK, /* disable controller integrity checking */
BIP_DISK_NOCHECK, /* disable disk integrity checking */
+ BIP_IP_CHECKSUM,/* IP checksum */
};
static inline bool bip_get_flag(struct bio_integrity_payload *bip,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 4be0446a8817..9bf6f761f1ac 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1432,6 +1432,7 @@ enum blk_integrity_flags {
BLK_INTEGRITY_VERIFY= 1 0,
BLK_INTEGRITY_GENERATE = 1 1,
BLK_INTEGRITY_DISK = 1 2,
+ BLK_INTEGRITY_IP_CHECKSUM = 1 3,
};
struct blk_integrity_iter {
--
1.9.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
[PATCH 14/14] sd: Honor block layer integrity handling flags
A set of flags introduced in the block layer enable better control over
how protection information is handled. These flags are useful for both
error injection and data recovery purposes. Checking can be enabled and
disabled for controller and disk, and the guard tag format is now a
per-I/O property.
Update sd_protect_op to communicate the relevant information to the
low-level device driver via a set of flags in scsi_cmnd.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
drivers/scsi/sd.c| 56
drivers/scsi/sd.h| 4 ++--
drivers/scsi/sd_dif.c| 55 +--
include/scsi/scsi_cmnd.h | 29 -
4 files changed, 96 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 96af195224f2..cd01dd8bed52 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -592,10 +592,11 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
mutex_unlock(sd_ref_mutex);
}
-static void sd_prot_op(struct scsi_cmnd *scmd, unsigned int dif)
+static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
+ unsigned int dix, unsigned int dif)
{
+ struct bio_integrity_payload *bip = bio_integrity(scmd-request-bio);
unsigned int prot_op = SCSI_PROT_NORMAL;
- unsigned int dix = scsi_prot_sg_count(scmd);
if (scmd-sc_data_direction == DMA_FROM_DEVICE) {
if (dif dix)
@@ -613,8 +614,37 @@ static void sd_prot_op(struct scsi_cmnd *scmd, unsigned
int dif)
prot_op = SCSI_PROT_WRITE_STRIP;
}
- scsi_set_prot_op(scmd, prot_op);
+ /* Let HBA know which protection type is used so it knows which
+* fields to check.
+*/
scsi_set_prot_type(scmd, dif);
+ scsi_set_prot_op(scmd, prot_op);
+
+ if (dix bip_get_flag(bip, BIP_IP_CHECKSUM))
+ scmd-prot_flags |= SCSI_PROT_IP_CHECKSUM;
+
+ if (dif != SD_DIF_TYPE3_PROTECTION)
+ scmd-prot_flags |= SCSI_PROT_REF_INCREMENT;
+
+ if (prot_op != SCSI_PROT_WRITE_INSERT
+ prot_op != SCSI_PROT_READ_INSERT
+ !bip_get_flag(bip, BIP_CTRL_NOCHECK)) {
+ scmd-prot_flags |= SCSI_PROT_GUARD_CHECK;
+
+ if (dif != SD_DIF_TYPE3_PROTECTION)
+ scmd-prot_flags |= SCSI_PROT_REF_CHECK;
+ }
+
+ if (dif) {
+ scmd-prot_flags |= SCSI_PROT_TRANSFER_PI;
+
+ if (bip_get_flag(bip, BIP_DISK_NOCHECK))
+ return 3 5; /* Transmit but do not check PI */
+ else
+ return 1 5; /* Transmit and check PI */
+ }
+
+ return 0;
}
static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode)
@@ -867,7 +897,8 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
sector_t block = blk_rq_pos(rq);
sector_t threshold;
unsigned int this_count = blk_rq_sectors(rq);
- int ret, host_dif;
+ unsigned int dif, dix;
+ int ret;
unsigned char protect;
/*
@@ -994,7 +1025,7 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
SCpnt-sc_data_direction = DMA_TO_DEVICE;
if (blk_integrity_rq(rq))
- sd_dif_prepare(rq, block, sdp-sector_size);
+ sd_dif_prepare(SCpnt);
} else if (rq_data_dir(rq) == READ) {
SCpnt-cmnd[0] = READ_6;
@@ -1010,14 +1041,15 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
writing : reading, this_count,
blk_rq_sectors(rq)));
- /* Set RDPROTECT/WRPROTECT if disk is formatted with DIF */
- host_dif = scsi_host_dif_capable(sdp-host, sdkp-protection_type);
- if (host_dif)
- protect = 1 5;
+ dix = scsi_prot_sg_count(SCpnt);
+ dif = scsi_host_dif_capable(SCpnt-device-host, sdkp-protection_type);
+
+ if (dif || dix)
+ protect = sd_setup_protect_cmnd(SCpnt, dix, dif);
else
protect = 0;
- if (host_dif == SD_DIF_TYPE2_PROTECTION) {
+ if (protect sdkp-protection_type == SD_DIF_TYPE2_PROTECTION) {
SCpnt-cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
if (unlikely(SCpnt-cmnd == NULL)) {
@@ -1105,10 +1137,6 @@ static int sd_prep_fn(struct request_queue *q, struct
request *rq)
}
SCpnt-sdb.length = this_count * sdp-sector_size;
- /* If DIF or DIX is enabled, tell HBA how to handle request */
- if (host_dif || scsi_prot_sg_count(SCpnt))
- sd_prot_op(SCpnt, host_dif);
-
/*
* We shouldn't disconnect in the middle of a sector, so with a dumb
* host adapter, it's safe to assume that we
[PATCH 1/6] block: Replace bi_integrity with bi_special
For commands like REQ_COPY we need a way to pass extra information along
with each bio. Like integrity metadata this information must be
available at the bottom of the stack so bi_private does not suffice.
Rename the existing bi_integrity field to bi_special and make it a union
so we can have different bio extensions for each class of command.
We previously used bi_integrity != NULL as a way to identify whether a
bio had integrity metadata or not. Introduce a REQ_INTEGRITY to be the
indicator now that bi_special can contain different things.
In addition, bio_integrity(bio) will now return a pointer to the
integrity payload (when applicable).
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/block/data-integrity.txt | 10 +-
block/bio-integrity.c | 23 ---
drivers/scsi/sd_dif.c | 8
include/linux/bio.h| 10 +++---
include/linux/blk_types.h | 8 ++--
include/linux/blkdev.h | 7 ++-
6 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/Documentation/block/data-integrity.txt
b/Documentation/block/data-integrity.txt
index 2d735b0ae383..5a0efc9ee5d5 100644
--- a/Documentation/block/data-integrity.txt
+++ b/Documentation/block/data-integrity.txt
@@ -129,11 +129,11 @@ interface for this is being worked on.
4.1 BIO
The data integrity patches add a new field to struct bio when
-CONFIG_BLK_DEV_INTEGRITY is enabled. bio-bi_integrity is a pointer
-to a struct bip which contains the bio integrity payload. Essentially
-a bip is a trimmed down struct bio which holds a bio_vec containing
-the integrity metadata and the required housekeeping information (bvec
-pool, vector count, etc.)
+CONFIG_BLK_DEV_INTEGRITY is enabled. bio_integrity(bio) returns a
+pointer to a struct bip which contains the bio integrity payload.
+Essentially a bip is a trimmed down struct bio which holds a bio_vec
+containing the integrity metadata and the required housekeeping
+information (bvec pool, vector count, etc.)
A kernel subsystem can enable data integrity protection on a bio by
calling bio_integrity_alloc(bio). This will allocate and attach the
diff --git a/block/bio-integrity.c b/block/bio-integrity.c
index 9e241063a616..40c6a0e50301 100644
--- a/block/bio-integrity.c
+++ b/block/bio-integrity.c
@@ -76,7 +76,8 @@ struct bio_integrity_payload *bio_integrity_alloc(struct bio
*bio,
bip-bip_slab = idx;
bip-bip_bio = bio;
- bio-bi_integrity = bip;
+ bio-bi_special.integrity = bip;
+ bio-bi_rw |= REQ_INTEGRITY;
return bip;
err:
@@ -94,7 +95,7 @@ EXPORT_SYMBOL(bio_integrity_alloc);
*/
void bio_integrity_free(struct bio *bio)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_set *bs = bio-bi_pool;
if (bip-bip_owns_buf)
@@ -110,7 +111,7 @@ void bio_integrity_free(struct bio *bio)
kfree(bip);
}
- bio-bi_integrity = NULL;
+ bio-bi_special.integrity = NULL;
}
EXPORT_SYMBOL(bio_integrity_free);
@@ -134,7 +135,7 @@ static inline unsigned int bip_integrity_vecs(struct
bio_integrity_payload *bip)
int bio_integrity_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int offset)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct bio_vec *iv;
if (bip-bip_vcnt = bip_integrity_vecs(bip)) {
@@ -240,7 +241,7 @@ EXPORT_SYMBOL(bio_integrity_tag_size);
static int bio_integrity_tag(struct bio *bio, void *tag_buf, unsigned int len,
int set)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
struct blk_integrity *bi = bdev_get_integrity(bio-bi_bdev);
unsigned int nr_sectors;
@@ -315,12 +316,12 @@ static int bio_integrity_generate_verify(struct bio *bio,
int operate)
struct bio_vec *bv;
sector_t sector;
unsigned int sectors, ret = 0, i;
- void *prot_buf = bio-bi_integrity-bip_buf;
+ void *prot_buf = bio_integrity(bio)-bip_buf;
if (operate)
sector = bio-bi_iter.bi_sector;
else
- sector = bio-bi_integrity-bip_iter.bi_sector;
+ sector = bio_integrity(bio)-bip_iter.bi_sector;
bix.disk_name = bio-bi_bdev-bd_disk-disk_name;
bix.sector_size = bi-sector_size;
@@ -516,7 +517,7 @@ static void bio_integrity_verify_fn(struct work_struct
*work)
*/
void bio_integrity_endio(struct bio *bio, int error)
{
- struct bio_integrity_payload *bip = bio-bi_integrity;
+ struct bio_integrity_payload *bip = bio_integrity(bio);
BUG_ON(bip-bip_bio != bio);
@@ -547,7 +548,7 @@
[PATCH 3/6] block: Introduce copy offload library function
blkdev_issue_copy() is a library function that filesystems can use to
clone block ranges between devices that support copy offloading. Both
source and target device must have max_copy_sectors 0 in the queue
limits.
blkdev_issue_copy() will iterate over the blocks in the source range and
issue copy offload requests using the granularity preferred by source
and target.
There is no guarantee that a copy offload operation will be successful
even if both devices are offload-capable. Filesystems must be prepared
to manually copy or punt to userland if the operation fails.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/blk-lib.c| 85 ++
include/linux/blkdev.h | 2 ++
2 files changed, 87 insertions(+)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 97a733cf3d5f..5a0afc6e933e 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -305,3 +305,88 @@ int blkdev_issue_zeroout(struct block_device *bdev,
sector_t sector,
return __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask);
}
EXPORT_SYMBOL(blkdev_issue_zeroout);
+
+/**
+ * blkdev_issue_copy - queue a copy same operation
+ * @src_bdev: source blockdev
+ * @src_sector:source sector
+ * @dst_bdev: destination blockdev
+ * @dst_sector: destination sector
+ * @nr_sects: number of sectors to copy
+ * @gfp_mask: memory allocation flags (for bio_alloc)
+ *
+ * Description:
+ *Copy a block range from source device to target device.
+ */
+int blkdev_issue_copy(struct block_device *src_bdev, sector_t src_sector,
+ struct block_device *dst_bdev, sector_t dst_sector,
+ unsigned int nr_sects, gfp_t gfp_mask)
+{
+ DECLARE_COMPLETION_ONSTACK(wait);
+ struct request_queue *sq = bdev_get_queue(src_bdev);
+ struct request_queue *dq = bdev_get_queue(dst_bdev);
+ unsigned int max_copy_sectors;
+ struct bio_batch bb;
+ int ret = 0;
+
+ if (!sq || !dq)
+ return -ENXIO;
+
+ max_copy_sectors = min(sq-limits.max_copy_sectors,
+ dq-limits.max_copy_sectors);
+
+ if (max_copy_sectors == 0)
+ return -EOPNOTSUPP;
+
+ atomic_set(bb.done, 1);
+ bb.flags = 1 BIO_UPTODATE;
+ bb.wait = wait;
+
+ while (nr_sects) {
+ struct bio *bio;
+ struct bio_copy *bc;
+ unsigned int chunk;
+
+ bc = kmalloc(sizeof(struct bio_copy), gfp_mask);
+ if (!bc) {
+ ret = -ENOMEM;
+ break;
+ }
+
+ bio = bio_alloc(gfp_mask, 1);
+ if (!bio) {
+ kfree(bc);
+ ret = -ENOMEM;
+ break;
+ }
+
+ chunk = min(nr_sects, max_copy_sectors);
+
+ bio-bi_iter.bi_sector = dst_sector;
+ bio-bi_iter.bi_size = chunk 9;
+ bio-bi_end_io = bio_batch_end_io;
+ bio-bi_bdev = dst_bdev;
+ bio-bi_private = bb;
+ bio-bi_special.copy = bc;
+
+ bc-bic_bdev = src_bdev;
+ bc-bic_sector = src_sector;
+
+ atomic_inc(bb.done);
+ submit_bio(REQ_WRITE | REQ_COPY, bio);
+
+ src_sector += chunk;
+ dst_sector += chunk;
+ nr_sects -= chunk;
+ }
+
+ /* Wait for bios in-flight */
+ if (!atomic_dec_and_test(bb.done))
+ wait_for_completion_io(wait);
+
+ if (!test_bit(BIO_UPTODATE, bb.flags))
+ ret = -ENOTSUPP;
+
+ return ret;
+}
+EXPORT_SYMBOL(blkdev_issue_copy);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 0d80e09251e6..d2fe99e6b3b8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1136,6 +1136,8 @@ extern int blkdev_issue_discard(struct block_device
*bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
extern int blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, struct page *page);
+extern int blkdev_issue_copy(struct block_device *, sector_t,
+ struct block_device *, sector_t, unsigned int, gfp_t);
extern int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask);
static inline int sb_issue_discard(struct super_block *sb, sector_t block,
--
1.9.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
[PATCH 6/6] [SCSI] sd: Implement copy offload support
Implement support for hardware copy offload. This initial implementation
only supports EXTENDED COPY(LID1). If need be we can add support for
LID4 or token copy at a later date.
If a device has the 3PC flag set in the standard INQUIRY response we'll
issue a RECEIVE COPY OPERATION PARAMETERS command. We require the device
to support two copy source/copy destination descriptors and one block to
block (0x02) segment descriptor. The device must support the NAA
identification descriptor (0xE4). If the device is capable we'll set the
queue limits to indicate that the device supports copy offload.
The copy block range limit can be overridden in scsi_disk's
max_copy_block sysfs attribute.
sd_setup_copy_command() is used to prepare any REQ_COPY requests. The
relevant descriptors are placed in a payload page akin to REQ_DISCARD.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
drivers/scsi/sd.c | 254 -
drivers/scsi/sd.h | 4 +
include/scsi/scsi_device.h | 1 +
3 files changed, 257 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 96af195224f2..071225f34d63 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -100,6 +100,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC);
static void sd_config_discard(struct scsi_disk *, unsigned int);
static void sd_config_write_same(struct scsi_disk *);
+static void sd_config_copy(struct scsi_disk *);
static int sd_revalidate_disk(struct gendisk *);
static void sd_unlock_native_capacity(struct gendisk *disk);
static int sd_probe(struct device *);
@@ -461,6 +462,48 @@ max_write_same_blocks_store(struct device *dev, struct
device_attribute *attr,
}
static DEVICE_ATTR_RW(max_write_same_blocks);
+static ssize_t
+max_copy_blocks_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+ struct scsi_disk *sdkp = to_scsi_disk(dev);
+
+ return snprintf(buf, 20, %u\n, sdkp-max_copy_blocks);
+}
+
+static ssize_t
+max_copy_blocks_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct scsi_disk *sdkp = to_scsi_disk(dev);
+ struct scsi_device *sdp = sdkp-device;
+ unsigned long max;
+ int err;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ if (sdp-type != TYPE_DISK)
+ return -EINVAL;
+
+ err = kstrtoul(buf, 10, max);
+
+ if (err)
+ return err;
+
+ if (max == 0)
+ sdp-no_copy = 1;
+ else if (max = SD_MAX_COPY_BLOCKS) {
+ sdp-no_copy = 0;
+ sdkp-max_copy_blocks = max;
+ }
+
+ sd_config_copy(sdkp);
+
+ return count;
+}
+static DEVICE_ATTR_RW(max_copy_blocks);
+
static struct attribute *sd_disk_attrs[] = {
dev_attr_cache_type.attr,
dev_attr_FUA.attr,
@@ -472,6 +515,7 @@ static struct attribute *sd_disk_attrs[] = {
dev_attr_thin_provisioning.attr,
dev_attr_provisioning_mode.attr,
dev_attr_max_write_same_blocks.attr,
+ dev_attr_max_copy_blocks.attr,
dev_attr_max_medium_access_timeouts.attr,
NULL,
};
@@ -826,6 +870,100 @@ static int sd_setup_write_same_cmnd(struct scsi_device
*sdp, struct request *rq)
return ret;
}
+static void sd_config_copy(struct scsi_disk *sdkp)
+{
+ struct request_queue *q = sdkp-disk-queue;
+ unsigned int logical_block_size = sdkp-device-sector_size;
+
+ if (sdkp-device-no_copy)
+ sdkp-max_copy_blocks = 0;
+
+ /* Segment descriptor 0x02 has a 64k block limit */
+ sdkp-max_copy_blocks = min(sdkp-max_copy_blocks,
+ (u32)SD_MAX_CSD2_BLOCKS);
+
+ blk_queue_max_copy_sectors(q, sdkp-max_copy_blocks *
+ (logical_block_size 9));
+}
+
+static int sd_setup_copy_cmnd(struct scsi_device *sdp, struct request *rq)
+{
+ struct scsi_device *src_sdp, *dst_sdp;
+ sector_t src_lba, dst_lba;
+ unsigned int nr_blocks, buf_len, nr_bytes = blk_rq_bytes(rq);
+ int ret;
+ struct bio *bio = rq-bio;
+ struct bio_copy *bic = bio_copy(bio);
+ struct page *page;
+ unsigned char *buf;
+
+ if (!bic)
+ return BLKPREP_KILL;
+
+ dst_sdp = scsi_disk(rq-rq_disk)-device;
+ src_sdp = scsi_disk(bic-bic_bdev-bd_disk)-device;
+
+ if (src_sdp-no_copy || dst_sdp-no_copy)
+ return BLKPREP_KILL;
+
+ if (src_sdp-sector_size != dst_sdp-sector_size)
+ return BLKPREP_KILL;
+
+ dst_lba = blk_rq_pos(rq) (ilog2(dst_sdp-sector_size) - 9);
+ src_lba = bic-bic_sector (ilog2(src_sdp-sector_size) - 9);
+ nr_blocks = blk_rq_sectors(rq) (ilog2(dst_sdp-sector_size) - 9);
+
+ page = alloc_page(GFP_ATOMIC | __GFP_ZERO);
+ if (!page)
+ return BLKPREP_DEFER;
+
+ buf =
[PATCH 5/6] [SCSI] Look up and store NAA if VPD page 0x83 is present
Copy offloading requires us to know the NAA descriptor for both source
target device. This descriptor is mandatory in the Device Identification
VPD page. Locate this descriptor in the returned VPD data so we don't
have to do lookups for every copy command.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
drivers/scsi/scsi.c| 57 ++
include/scsi/scsi_device.h | 2 ++
2 files changed, 59 insertions(+)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 88d46fe6bf98..7faea9987abf 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -1024,6 +1024,62 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page,
unsigned char *buf,
EXPORT_SYMBOL_GPL(scsi_get_vpd_page);
/**
+ * scsi_lookup_naa - Lookup NAA descriptor in VPD page 0x83
+ * @sdev: The device to ask
+ *
+ * Copy offloading requires us to know the NAA descriptor for both
+ * source and target device. This descriptor is mandatory in the Device
+ * Identification VPD page. Locate this descriptor in the returned VPD
+ * data so we don't have to do lookups for every copy command.
+ */
+static void scsi_lookup_naa(struct scsi_device *sdev)
+{
+ unsigned char *buf = sdev-vpd_pg83;
+ unsigned int len = sdev-vpd_pg83_len;
+
+ if (buf[1] != 0x83 || get_unaligned_be16(buf[2]) == 0) {
+ sdev_printk(KERN_ERR, sdev,
+ %s: VPD page 0x83 contains no descriptors\n,
+ __func__);
+ return;
+ }
+
+ buf += 4;
+ len -= 4;
+
+ do {
+ unsigned int desig_len = buf[3] + 4;
+
+ /* Binary code set */
+ if ((buf[0] 0xf) != 1)
+ goto skip;
+
+ /* Target association */
+ if ((buf[1] 4) 0x3)
+ goto skip;
+
+ /* NAA designator */
+ if ((buf[1] 0xf) != 0x3)
+ goto skip;
+
+ sdev-naa = buf;
+ sdev-naa_len = desig_len;
+
+ return;
+
+skip:
+ buf += desig_len;
+ len -= desig_len;
+
+ } while (len 0);
+
+ sdev_printk(KERN_ERR, sdev,
+ %s: VPD page 0x83 NAA descriptor not found\n, __func__);
+
+ return;
+}
+
+/**
* scsi_attach_vpd - Attach Vital Product Data to a SCSI device structure
* @sdev: The device to ask
*
@@ -1107,6 +1163,7 @@ retry_pg83:
}
sdev-vpd_pg83_len = result;
sdev-vpd_pg83 = vpd_buf;
+ scsi_lookup_naa(sdev);
}
}
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 5853c913d2b0..67bb70012802 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -119,6 +119,8 @@ struct scsi_device {
unsigned char *vpd_pg83;
int vpd_pg80_len;
unsigned char *vpd_pg80;
+ unsigned char naa_len;
+ unsigned char *naa;
unsigned char current_tag; /* current tag */
struct scsi_target *sdev_target; /* used only for single_lun */
--
1.9.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
Copy offload
Several people have poked me about the copy offload patches. Aside from being able to fit into Jens' 3.16/core branch there hasn't been any changes, nor any bug reports since LSF/MM. This patch series implements support for copy offload. Storage arrays that implement copy operations can be told to clone a block range either within a storage device or between LUNs. The copy is done entirely inside the device and no data is moved back and forth across the wire. [PATCH 1/6] block: Replace bi_integrity with bi_special Identical to the patch I just posted in the integrity series. Allows us to have other uses for the pointer previously exclusively used for the integrity payload. [PATCH 2/6] block: Implement support for copy offload operations General block layer plumbing for REQ_COPY requests. [PATCH 3/6] block: Introduce copy offload library function Helper for filesystems that want to clone block ranges. [PATCH 4/6] block: Copy offload ioctl The ioctl I have been using to test the code. [PATCH 5/6] [SCSI] Look up and store NAA if VPD page 0x83 is present Leverages Hannes' VPD page code to store a pointer to the NAA descriptor. [PATCH 6/6] [SCSI] sd: Implement copy offload support The EXTENDED COPY implementation in the SCSI disk driver. Documentation/ABI/testing/sysfs-block |9 + Documentation/block/data-integrity.txt | 10 - block/bio-integrity.c | 23 +- block/blk-core.c |5 block/blk-lib.c| 85 +++ block/blk-merge.c |7 block/blk-settings.c | 15 + block/blk-sysfs.c | 10 + block/ioctl.c | 35 drivers/scsi/scsi.c| 57 +++ drivers/scsi/sd.c | 254 - drivers/scsi/sd.h |4 drivers/scsi/sd_dif.c |8 - include/linux/bio.h| 25 ++- include/linux/blk_types.h | 21 ++ include/linux/blkdev.h | 22 ++ include/scsi/scsi_device.h |3 include/uapi/linux/fs.h|1 18 files changed, 553 insertions(+), 41 deletions(-) -- 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
[PATCH 2/6] block: Implement support for copy offload operations
Many modern SCSI devices support copy offloading operations in which one
can copy a block range from one LUN to another without the need for data
to be copied sent to the host and back. This is particularly useful for
things like cloning LUNs or virtual machine images.
Implement support for REQ_COPY commands in the block layer:
- Add max_copy_sectors queue limits and handle stacking
- Expose this parameter in sysfs in bytes (copy_max_bytes)
- Add special casing for REQ_COPY in merging and mapping functions
- Introduce a bio_copy descriptor hanging off of bio-bi_special. This
descriptor contains the source bdev and source sector for the copy
operation. Target bdev/sector/size are described by the bio proper.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
Documentation/ABI/testing/sysfs-block | 9 +
block/blk-core.c | 5 +
block/blk-merge.c | 7 ++-
block/blk-settings.c | 15 +++
block/blk-sysfs.c | 10 ++
include/linux/bio.h | 15 +--
include/linux/blk_types.h | 15 ---
include/linux/blkdev.h| 13 +
8 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block
b/Documentation/ABI/testing/sysfs-block
index 279da08f7541..d1304cc305f7 100644
--- a/Documentation/ABI/testing/sysfs-block
+++ b/Documentation/ABI/testing/sysfs-block
@@ -220,3 +220,12 @@ Description:
write_same_max_bytes is 0, write same is not supported
by the device.
+
+What: /sys/block/disk/queue/copy_max_bytes
+Date: January 2014
+Contact: Martin K. Petersen martin.peter...@oracle.com
+Description:
+ Devices that support copy offloading will set this value
+ to indicate the maximum buffer size in bytes that can be
+ copied in one operation. If the copy_max_bytes is 0 the
+ device does not support copy offload.
diff --git a/block/blk-core.c b/block/blk-core.c
index 5b6f768a7c01..3a91044ee19b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1810,6 +1810,11 @@ generic_make_request_checks(struct bio *bio)
goto end_io;
}
+ if (bio-bi_rw REQ_COPY !bdev_copy_offload(bio-bi_bdev)) {
+ err = -EOPNOTSUPP;
+ goto end_io;
+ }
+
/*
* Various block parts want %current-io_context and lazy ioc
* allocation ends up trading a lot of pain for a small amount of
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 6c583f9c5b65..0e1027e2e32b 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -25,10 +25,7 @@ static unsigned int __blk_recalc_rq_segments(struct
request_queue *q,
* This should probably be returning 0, but blk_add_request_payload()
* (Christoph)
*/
- if (bio-bi_rw REQ_DISCARD)
- return 1;
-
- if (bio-bi_rw REQ_WRITE_SAME)
+ if (bio-bi_rw (REQ_DISCARD | REQ_WRITE_SAME | REQ_COPY))
return 1;
fbio = bio;
@@ -182,7 +179,7 @@ static int __blk_bios_map_sg(struct request_queue *q,
struct bio *bio,
nsegs = 0;
cluster = blk_queue_cluster(q);
- if (bio-bi_rw REQ_DISCARD) {
+ if (bio-bi_rw (REQ_DISCARD | REQ_COPY)) {
/*
* This is a hack - drivers should be neither modifying the
* biovec, nor relying on bi_vcnt - but because of
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 5d21239bc859..98801bcc02b0 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -114,6 +114,7 @@ void blk_set_default_limits(struct queue_limits *lim)
lim-max_segment_size = BLK_MAX_SEGMENT_SIZE;
lim-max_sectors = lim-max_hw_sectors = BLK_SAFE_MAX_SECTORS;
lim-max_write_same_sectors = 0;
+ lim-max_copy_sectors = 0;
lim-max_discard_sectors = 0;
lim-discard_granularity = 0;
lim-discard_alignment = 0;
@@ -147,6 +148,7 @@ void blk_set_stacking_limits(struct queue_limits *lim)
lim-max_segment_size = UINT_MAX;
lim-max_sectors = UINT_MAX;
lim-max_write_same_sectors = UINT_MAX;
+ lim-max_copy_sectors = UINT_MAX;
}
EXPORT_SYMBOL(blk_set_stacking_limits);
@@ -301,6 +303,18 @@ void blk_queue_max_write_same_sectors(struct request_queue
*q,
EXPORT_SYMBOL(blk_queue_max_write_same_sectors);
/**
+ * blk_queue_max_copy_sectors - set max sectors for a single copy operation
+ * @q: the request queue for the device
+ * @max_copy_sectors: maximum number of sectors per copy operation
+ **/
+void blk_queue_max_copy_sectors(struct request_queue *q,
+ unsigned int max_copy_sectors)
+{
+ q-limits.max_copy_sectors = max_copy_sectors;
+}
+EXPORT_SYMBOL(blk_queue_max_copy_sectors);
+
+/**
*
[PATCH 4/6] block: Copy offload ioctl
Add an ioctl which can be used to clone a block range within a single
block device. This is useful for testing the copy offload code.
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com
---
block/ioctl.c | 35 +++
include/uapi/linux/fs.h | 1 +
2 files changed, 36 insertions(+)
diff --git a/block/ioctl.c b/block/ioctl.c
index 7d5c3b20af45..5efb6e666f18 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -201,6 +201,29 @@ static int blk_ioctl_zeroout(struct block_device *bdev,
uint64_t start,
return blkdev_issue_zeroout(bdev, start, len, GFP_KERNEL);
}
+static int blk_ioctl_copy(struct block_device *bdev, uint64_t src_offset,
+ uint64_t dst_offset, uint64_t len)
+{
+ if (src_offset 511)
+ return -EINVAL;
+ if (dst_offset 511)
+ return -EINVAL;
+ if (len 511)
+ return -EINVAL;
+ src_offset = 9;
+ dst_offset = 9;
+ len = 9;
+
+ if (src_offset + len (i_size_read(bdev-bd_inode) 9))
+ return -EINVAL;
+
+ if (dst_offset + len (i_size_read(bdev-bd_inode) 9))
+ return -EINVAL;
+
+ return blkdev_issue_copy(bdev, src_offset, bdev, dst_offset, len,
+GFP_KERNEL);
+}
+
static int put_ushort(unsigned long arg, unsigned short val)
{
return put_user(val, (unsigned short __user *)arg);
@@ -328,6 +351,18 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode,
unsigned cmd,
return blk_ioctl_zeroout(bdev, range[0], range[1]);
}
+ case BLKCOPY: {
+ uint64_t range[3];
+
+ if (!(mode FMODE_WRITE))
+ return -EBADF;
+
+ if (copy_from_user(range, (void __user *)arg, sizeof(range)))
+ return -EFAULT;
+
+ return blk_ioctl_copy(bdev, range[0], range[1], range[2]);
+ }
+
case HDIO_GETGEO: {
struct hd_geometry geo;
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index ca1a11bb4443..195c2c4cbacc 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -149,6 +149,7 @@ struct inodes_stat_t {
#define BLKSECDISCARD _IO(0x12,125)
#define BLKROTATIONAL _IO(0x12,126)
#define BLKZEROOUT _IO(0x12,127)
+#define BLKCOPY _IO(0x12,128)
#define BMAP_IOCTL 1 /* obsolete - kept for compatibility */
#define FIBMAP_IO(0x00,1) /* bmap access */
--
1.9.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
