[PATCH-v2 0/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation

[PATCH-v2 0/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi folks,

This -v2 series for-3.8 adds support for proper WRITE_SAME w/ UNMAP=0 emulation
for IBLOCK device backends to follow MKP's WRITE_SAME client patches that have
been merged in v3.7-rc1.

Currently it uses a bio_add_page() call for each sector in order to allow
scatterlist w/ page offsets to work, as blkdev_issue_write_same() currently
assumes underlying hw support + zero page offset.

This -v2 includes the following changes requested by hch to v1:

 - Drop the SCF_WRITE_SAME_DISCARD flag
 - Add sbc_ops handler specific to WRITE_SAME w/ UNMAP=1
 - Move max_write_same_len sanity check from IBLOCK into sbc code
 - Add SGL check for single sector in iblock_execute_write_same()

I'm testing these changes now against iscsi-target + tcm_loop ports on
target-pending/for-next.  Please review.

Thanks!

--nab

Nicholas Bellinger (3):
  target/sbc: Seperate WRITE_SAME based on UNMAP flag in sbc_ops
  target: Add/check max_write_same_len device attribute + update block
    limits VPD
  target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation support

 drivers/target/target_core_configfs.c |    4 ++
 drivers/target/target_core_device.c   |   11 ++++
 drivers/target/target_core_iblock.c   |   91 ++++++++++++++++++++++++++++++---
 drivers/target/target_core_internal.h |    1 +
 drivers/target/target_core_sbc.c      |   59 +++++++++++----------
 drivers/target/target_core_spc.c      |    8 +++-
 include/target/target_core_backend.h  |    1 +
 include/target/target_core_base.h     |    3 +
 8 files changed, 142 insertions(+), 36 deletions(-)

-- 
1.7.2.5

[PATCH-v2 1/3] target/sbc: Seperate WRITE_SAME based on UNMAP flag in sbc_ops

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds a new sbc_ops->execute_write_same_unmap() caller for use
with WRITE_SAME w/ UNMAP=1, and performs the ->execute_cmd() setup based
this bit within sbc_setup_write_same() code.

Also, makes the changes in sbc_parse_cdb() to handle a sense_reason_t
return from sbc_setup_write_same() on error.

Reported-by: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_sbc.c     |   51 +++++++++++++++------------------
 include/target/target_core_backend.h |    1 +
 2 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index b802421..a5a8f46 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -235,26 +235,30 @@ static inline unsigned long long transport_lba_64_ext(unsigned char *cdb)
 	return ((unsigned long long)__v2) | (unsigned long long)__v1 << 32;
 }
 
-static int sbc_write_same_supported(struct se_device *dev,
-		unsigned char *flags)
+static sense_reason_t
+sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *ops)
 {
 	if ((flags[0] & 0x04) || (flags[0] & 0x02)) {
 		pr_err("WRITE_SAME PBDATA and LBDATA"
 			" bits not supported for Block Discard"
 			" Emulation\n");
-		return -ENOSYS;
+		return TCM_UNSUPPORTED_SCSI_OPCODE;
 	}
-
 	/*
-	 * Currently for the emulated case we only accept
-	 * tpws with the UNMAP=1 bit set.
+	 * Special case for WRITE_SAME w/ UNMAP=1 that ends up getting
+	 * translated into block discard requests within backend code.
 	 */
-	if (!(flags[0] & 0x08)) {
-		pr_err("WRITE_SAME w/o UNMAP bit not"
-			" supported for Block Discard Emulation\n");
-		return -ENOSYS;
+	if (flags[0] & 0x08) {
+		if (!ops->execute_write_same_unmap)
+			return TCM_UNSUPPORTED_SCSI_OPCODE;
+
+		cmd->execute_cmd = ops->execute_write_same_unmap;
+		return 0;
 	}
+	if (!ops->execute_write_same)
+		return TCM_UNSUPPORTED_SCSI_OPCODE;
 
+	cmd->execute_cmd = ops->execute_write_same;
 	return 0;
 }
 
@@ -418,9 +422,6 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 				cmd->se_cmd_flags |= SCF_FUA;
 			break;
 		case WRITE_SAME_32:
-			if (!ops->execute_write_same)
-				return TCM_UNSUPPORTED_SCSI_OPCODE;
-
 			sectors = transport_get_sectors_32(cdb);
 			if (!sectors) {
 				pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not"
@@ -431,9 +432,9 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 			size = sbc_get_size(cmd, 1);
 			cmd->t_task_lba = get_unaligned_be64(&cdb[12]);
 
-			if (sbc_write_same_supported(dev, &cdb[10]) < 0)
-				return TCM_UNSUPPORTED_SCSI_OPCODE;
-			cmd->execute_cmd = ops->execute_write_same;
+			ret = sbc_setup_write_same(cmd, &cdb[10], ops);
+			if (ret < 0)
+				return ret;
 			break;
 		default:
 			pr_err("VARIABLE_LENGTH_CMD service action"
@@ -495,9 +496,6 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 		cmd->execute_cmd = ops->execute_unmap;
 		break;
 	case WRITE_SAME_16:
-		if (!ops->execute_write_same)
-			return TCM_UNSUPPORTED_SCSI_OPCODE;
-
 		sectors = transport_get_sectors_16(cdb);
 		if (!sectors) {
 			pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not supported\n");
@@ -507,14 +505,11 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 		size = sbc_get_size(cmd, 1);
 		cmd->t_task_lba = get_unaligned_be64(&cdb[2]);
 
-		if (sbc_write_same_supported(dev, &cdb[1]) < 0)
-			return TCM_UNSUPPORTED_SCSI_OPCODE;
-		cmd->execute_cmd = ops->execute_write_same;
+		ret = sbc_setup_write_same(cmd, &cdb[1], ops);
+		if (ret < 0)
+			return ret;
 		break;
 	case WRITE_SAME:
-		if (!ops->execute_write_same)
-			return TCM_UNSUPPORTED_SCSI_OPCODE;
-
 		sectors = transport_get_sectors_10(cdb);
 		if (!sectors) {
 			pr_err("WSNZ=1, WRITE_SAME w/sectors=0 not supported\n");
@@ -528,9 +523,9 @@ sbc_parse_cdb(struct se_cmd *cmd, struct sbc_ops *ops)
 		 * Follow sbcr26 with WRITE_SAME (10) and check for the existence
 		 * of byte 1 bit 3 UNMAP instead of original reserved field
 		 */
-		if (sbc_write_same_supported(dev, &cdb[1]) < 0)
-			return TCM_UNSUPPORTED_SCSI_OPCODE;
-		cmd->execute_cmd = ops->execute_write_same;
+		ret = sbc_setup_write_same(cmd, &cdb[1], ops);
+		if (ret < 0)
+			return ret;
 		break;
 	case VERIFY:
 		size = 0;
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index aa86809..3393ab1 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -41,6 +41,7 @@ struct sbc_ops {
 	sense_reason_t (*execute_rw)(struct se_cmd *cmd);
 	sense_reason_t (*execute_sync_cache)(struct se_cmd *cmd);
 	sense_reason_t (*execute_write_same)(struct se_cmd *cmd);
+	sense_reason_t (*execute_write_same_unmap)(struct se_cmd *cmd);
 	sense_reason_t (*execute_unmap)(struct se_cmd *cmd);
 };
 
-- 
1.7.2.5

[PATCH-v2 2/3] target: Add/check max_write_same_len device attribute + update block limits VPD

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds a new max_write_same_len device attribute for use with
WRITE_SAME w/ UNMAP=0 backend emulation.  This can be useful for
lowering the default backend value (IBLOCK uses 0xFFFF).

Also, update block limits VPD emulation code in spc_emulate_evpd_b0() to
report MAXIMUM WRITE SAME LENGTH, and enforce max_write_same_len during
sbc_parse() -> sbc_setup_write_same() CDB sanity checking for all emulated
WRITE_SAME w/ UNMAP=0 cases.

Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_configfs.c |    4 ++++
 drivers/target/target_core_device.c   |   11 +++++++++++
 drivers/target/target_core_internal.h |    1 +
 drivers/target/target_core_sbc.c      |    8 ++++++++
 drivers/target/target_core_spc.c      |    8 +++++++-
 include/target/target_core_base.h     |    3 +++
 6 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 7b473b6..2b14164 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -676,6 +676,9 @@ SE_DEV_ATTR(unmap_granularity, S_IRUGO | S_IWUSR);
 DEF_DEV_ATTRIB(unmap_granularity_alignment);
 SE_DEV_ATTR(unmap_granularity_alignment, S_IRUGO | S_IWUSR);
 
+DEF_DEV_ATTRIB(max_write_same_len);
+SE_DEV_ATTR(max_write_same_len, S_IRUGO | S_IWUSR);
+
 CONFIGFS_EATTR_OPS(target_core_dev_attrib, se_dev_attrib, da_group);
 
 static struct configfs_attribute *target_core_dev_attrib_attrs[] = {
@@ -701,6 +704,7 @@ static struct configfs_attribute *target_core_dev_attrib_attrs[] = {
 	&target_core_dev_attrib_max_unmap_block_desc_count.attr,
 	&target_core_dev_attrib_unmap_granularity.attr,
 	&target_core_dev_attrib_unmap_granularity_alignment.attr,
+	&target_core_dev_attrib_max_write_same_len.attr,
 	NULL,
 };
 
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 599374e..54439bc 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -706,6 +706,16 @@ int se_dev_set_unmap_granularity_alignment(
 	return 0;
 }
 
+int se_dev_set_max_write_same_len(
+	struct se_device *dev,
+	u32 max_write_same_len)
+{
+	dev->dev_attrib.max_write_same_len = max_write_same_len;
+	pr_debug("dev[%p]: Set max_write_same_len: %u\n",
+			dev, dev->dev_attrib.max_write_same_len);
+	return 0;
+}
+
 int se_dev_set_emulate_dpo(struct se_device *dev, int flag)
 {
 	if (flag != 0 && flag != 1) {
@@ -1393,6 +1403,7 @@ struct se_device *target_alloc_device(struct se_hba *hba, const char *name)
 	dev->dev_attrib.unmap_granularity = DA_UNMAP_GRANULARITY_DEFAULT;
 	dev->dev_attrib.unmap_granularity_alignment =
 				DA_UNMAP_GRANULARITY_ALIGNMENT_DEFAULT;
+	dev->dev_attrib.max_write_same_len = DA_MAX_WRITE_SAME_LEN;
 	dev->dev_attrib.fabric_max_sectors = DA_FABRIC_MAX_SECTORS;
 	dev->dev_attrib.optimal_sectors = DA_FABRIC_MAX_SECTORS;
 
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index bc9c522..93e9c1f 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -24,6 +24,7 @@ int	se_dev_set_max_unmap_lba_count(struct se_device *, u32);
 int	se_dev_set_max_unmap_block_desc_count(struct se_device *, u32);
 int	se_dev_set_unmap_granularity(struct se_device *, u32);
 int	se_dev_set_unmap_granularity_alignment(struct se_device *, u32);
+int	se_dev_set_max_write_same_len(struct se_device *, u32);
 int	se_dev_set_emulate_dpo(struct se_device *, int);
 int	se_dev_set_emulate_fua_write(struct se_device *, int);
 int	se_dev_set_emulate_fua_read(struct se_device *, int);
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index a5a8f46..8dd8a802 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -238,6 +238,8 @@ static inline unsigned long long transport_lba_64_ext(unsigned char *cdb)
 static sense_reason_t
 sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *ops)
 {
+	unsigned int sectors = spc_get_write_same_sectors(cmd);
+
 	if ((flags[0] & 0x04) || (flags[0] & 0x02)) {
 		pr_err("WRITE_SAME PBDATA and LBDATA"
 			" bits not supported for Block Discard"
@@ -258,6 +260,12 @@ sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *o
 	if (!ops->execute_write_same)
 		return TCM_UNSUPPORTED_SCSI_OPCODE;
 
+	if (sectors > cmd->se_dev->dev_attrib.max_write_same_len) {
+		pr_warn("WRITE_SAME sectors: %u exceeds max_write_same_len: %u\n",
+			sectors, cmd->se_dev->dev_attrib.max_write_same_len);
+		return TCM_INVALID_CDB_FIELD;
+	}
+
 	cmd->execute_cmd = ops->execute_write_same;
 	return 0;
 }
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 4b3c183..cf1b8bb 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -465,7 +465,7 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
 	 * Exit now if we don't support TP.
 	 */
 	if (!have_tp)
-		return 0;
+		goto max_write_same;
 
 	/*
 	 * Set MAXIMUM UNMAP LBA COUNT
@@ -491,6 +491,12 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned char *buf)
 	if (dev->dev_attrib.unmap_granularity_alignment != 0)
 		buf[32] |= 0x80; /* Set the UGAVALID bit */
 
+	/*
+	 * MAXIMUM WRITE SAME LENGTH
+	 */
+max_write_same:
+	put_unaligned_be64(dev->dev_attrib.max_write_same_len, &buf[36]);
+
 	return 0;
 }
 
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 5350f6e..645d90a 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -71,6 +71,8 @@
 #define DA_UNMAP_GRANULARITY_DEFAULT		0
 /* Default unmap_granularity_alignment */
 #define DA_UNMAP_GRANULARITY_ALIGNMENT_DEFAULT	0
+/* Default max_write_same_len, disabled by default */
+#define DA_MAX_WRITE_SAME_LEN			0
 /* Default max transfer length */
 #define DA_FABRIC_MAX_SECTORS			8192
 /* Emulation for Direct Page Out */
@@ -609,6 +611,7 @@ struct se_dev_attrib {
 	u32		max_unmap_block_desc_count;
 	u32		unmap_granularity;
 	u32		unmap_granularity_alignment;
+	u32		max_write_same_len;
 	struct se_device *da_dev;
 	struct config_group da_group;
 };
-- 
1.7.2.5

[PATCH-v2 3/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation support

[Nicholas A. Bellinger]

From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch adds support for emulation of WRITE_SAME w/ UNMAP=0 within
iblock_execute_write_same() backend code.

The emulation uses a bio_add_page() call for each sector, and by default
enforces a limit of max_write_same_len=0xFFFF (65536) sectors following
what scsi_debug reports per default for MAXIMUM WRITE SAME LENGTH.

It also sets max_write_same_len to the operational default at setup ->
iblock_configure_device() time.

(hch: Move unmap logic into iblock_execute_write_same_unmap + add
      check for single sector SGLs in iblock_execute_write_same)
(mkp: Update comment for 0xFFFF magic constant)

Cc: Christoph Hellwig <hch@lst.de>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_iblock.c |   91 ++++++++++++++++++++++++++++++++---
 1 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 53f4501..5abbd55 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -151,6 +151,11 @@ static int iblock_configure_device(struct se_device *dev)
 		pr_debug("IBLOCK: BLOCK Discard support available,"
 				" disabled by default\n");
 	}
+	/*
+	 * Enable write same emulation for IBLOCK and use 0xFFFF as
+	 * the samller WRITE_SAME(10) only has a two-byte block count.
+	 */
+	dev->dev_attrib.max_write_same_len = 0xFFFF;
 
 	if (blk_queue_nonrot(q))
 		dev->dev_attrib.is_nonrot = 1;
@@ -375,17 +380,20 @@ err:
 	return ret;
 }
 
+static struct bio *iblock_get_bio(struct se_cmd *, sector_t, u32);
+static void iblock_submit_bios(struct bio_list *, int);
+static void iblock_complete_cmd(struct se_cmd *);
+
 static sense_reason_t
-iblock_execute_write_same(struct se_cmd *cmd)
+iblock_execute_write_same_unmap(struct se_cmd *cmd)
 {
 	struct iblock_dev *ib_dev = IBLOCK_DEV(cmd->se_dev);
-	int ret;
+	int rc;
 
-	ret = blkdev_issue_discard(ib_dev->ibd_bd, cmd->t_task_lba,
-				   spc_get_write_same_sectors(cmd), GFP_KERNEL,
-				   0);
-	if (ret < 0) {
-		pr_debug("blkdev_issue_discard() failed for WRITE_SAME\n");
+	rc = blkdev_issue_discard(ib_dev->ibd_bd, cmd->t_task_lba,
+			spc_get_write_same_sectors(cmd), GFP_KERNEL, 0);
+	if (rc < 0) {
+		pr_warn("blkdev_issue_discard() failed: %d\n", rc);
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
@@ -393,6 +401,74 @@ iblock_execute_write_same(struct se_cmd *cmd)
 	return 0;
 }
 
+static sense_reason_t
+iblock_execute_write_same(struct se_cmd *cmd)
+{
+	struct iblock_req *ibr;
+	struct scatterlist *sg;
+	struct bio *bio;
+	struct bio_list list;
+	sector_t block_lba = cmd->t_task_lba;
+	unsigned int sectors = spc_get_write_same_sectors(cmd);
+
+	if (sectors > cmd->se_dev->dev_attrib.max_write_same_len) {
+		pr_warn("WRITE_SAME sectors: %u exceeds max_write_same_len: %u\n",
+			sectors, cmd->se_dev->dev_attrib.max_write_same_len);
+		return TCM_INVALID_CDB_FIELD;
+	}
+	sg = &cmd->t_data_sg[0];
+
+	if (cmd->t_data_nents > 1 ||
+	    sg->length != cmd->se_dev->dev_attrib.block_size) {
+		pr_err("WRITE_SAME: Illegal SGL t_data_nents: %u length: %u"
+			" block_size: %u\n", cmd->t_data_nents, sg->length,
+			cmd->se_dev->dev_attrib.block_size);
+		return TCM_INVALID_CDB_FIELD;
+	}
+
+	ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL);
+	if (!ibr)
+		goto fail;
+	cmd->priv = ibr;
+
+	bio = iblock_get_bio(cmd, block_lba, 1);
+	if (!bio)
+		goto fail_free_ibr;
+
+	bio_list_init(&list);
+	bio_list_add(&list, bio);
+
+	atomic_set(&ibr->pending, 1);
+
+	while (sectors) {
+		while (bio_add_page(bio, sg_page(sg), sg->length, sg->offset)
+				!= sg->length) {
+
+			bio = iblock_get_bio(cmd, block_lba, 1);
+			if (!bio)
+				goto fail_put_bios;
+
+			atomic_inc(&ibr->pending);
+			bio_list_add(&list, bio);
+		}
+
+		/* Always in 512 byte units for Linux/Block */
+		block_lba += sg->length >> IBLOCK_LBA_SHIFT;
+		sectors -= 1;
+	}
+
+	iblock_submit_bios(&list, WRITE);
+	return 0;
+
+fail_put_bios:
+	while ((bio = bio_list_pop(&list)))
+		bio_put(bio);
+fail_free_ibr:
+	kfree(ibr);
+fail:
+	return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+}
+
 enum {
 	Opt_udev_path, Opt_readonly, Opt_force, Opt_err
 };
@@ -701,6 +777,7 @@ static struct sbc_ops iblock_sbc_ops = {
 	.execute_rw		= iblock_execute_rw,
 	.execute_sync_cache	= iblock_execute_sync_cache,
 	.execute_write_same	= iblock_execute_write_same,
+	.execute_write_same_unmap = iblock_execute_write_same_unmap,
 	.execute_unmap		= iblock_execute_unmap,
 };
 
-- 
1.7.2.5

Re: [PATCH-v2 2/3] target: Add/check max_write_same_len device attribute + update block limits VPD

[Nicholas A. Bellinger]

On Thu, 2012-11-15 at 20:13 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch adds a new max_write_same_len device attribute for use with
> WRITE_SAME w/ UNMAP=0 backend emulation.  This can be useful for
> lowering the default backend value (IBLOCK uses 0xFFFF).
> 
> Also, update block limits VPD emulation code in spc_emulate_evpd_b0() to
> report MAXIMUM WRITE SAME LENGTH, and enforce max_write_same_len during
> sbc_parse() -> sbc_setup_write_same() CDB sanity checking for all emulated
> WRITE_SAME w/ UNMAP=0 cases.
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/target_core_configfs.c |    4 ++++
>  drivers/target/target_core_device.c   |   11 +++++++++++
>  drivers/target/target_core_internal.h |    1 +
>  drivers/target/target_core_sbc.c      |    8 ++++++++
>  drivers/target/target_core_spc.c      |    8 +++++++-
>  include/target/target_core_base.h     |    3 +++
>  6 files changed, 34 insertions(+), 1 deletions(-)
> 

<SNIP>

> diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
> index a5a8f46..8dd8a802 100644
> --- a/drivers/target/target_core_sbc.c
> +++ b/drivers/target/target_core_sbc.c
> @@ -238,6 +238,8 @@ static inline unsigned long long transport_lba_64_ext(unsigned char *cdb)
>  static sense_reason_t
>  sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *ops)
>  {
> +	unsigned int sectors = spc_get_write_same_sectors(cmd);
> +
>  	if ((flags[0] & 0x04) || (flags[0] & 0x02)) {
>  		pr_err("WRITE_SAME PBDATA and LBDATA"
>  			" bits not supported for Block Discard"
> @@ -258,6 +260,12 @@ sbc_setup_write_same(struct se_cmd *cmd, unsigned char *flags, struct sbc_ops *o
>  	if (!ops->execute_write_same)
>  		return TCM_UNSUPPORTED_SCSI_OPCODE;
>  
> +	if (sectors > cmd->se_dev->dev_attrib.max_write_same_len) {
> +		pr_warn("WRITE_SAME sectors: %u exceeds max_write_same_len: %u\n",
> +			sectors, cmd->se_dev->dev_attrib.max_write_same_len);
> +		return TCM_INVALID_CDB_FIELD;
> +	}
> +
>  	cmd->execute_cmd = ops->execute_write_same;
>  	return 0;
>  }

Now moving the max_write_same_len check above directly ahead of the
UNMAP conditional branch, so that both WRITE_SAME w/ UNMAP=[1,0] are
properly checked against this value as per hch's + Robert's input.

Thanks!

--nab

Re: [PATCH-v2 3/3] target/iblock: Add WRITE_SAME w/ UNMAP=0 emulation support

[Nicholas A. Bellinger]

On Thu, 2012-11-15 at 20:13 +0000, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> This patch adds support for emulation of WRITE_SAME w/ UNMAP=0 within
> iblock_execute_write_same() backend code.
> 
> The emulation uses a bio_add_page() call for each sector, and by default
> enforces a limit of max_write_same_len=0xFFFF (65536) sectors following
> what scsi_debug reports per default for MAXIMUM WRITE SAME LENGTH.
> 
> It also sets max_write_same_len to the operational default at setup ->
> iblock_configure_device() time.
> 
> (hch: Move unmap logic into iblock_execute_write_same_unmap + add
>       check for single sector SGLs in iblock_execute_write_same)
> (mkp: Update comment for 0xFFFF magic constant)
> 
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
> ---
>  drivers/target/target_core_iblock.c |   91 ++++++++++++++++++++++++++++++++---
>  1 files changed, 84 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
> index 53f4501..5abbd55 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c

> +static sense_reason_t
> +iblock_execute_write_same(struct se_cmd *cmd)
> +{
> +	struct iblock_req *ibr;
> +	struct scatterlist *sg;
> +	struct bio *bio;
> +	struct bio_list list;
> +	sector_t block_lba = cmd->t_task_lba;
> +	unsigned int sectors = spc_get_write_same_sectors(cmd);
> +
> +	if (sectors > cmd->se_dev->dev_attrib.max_write_same_len) {
> +		pr_warn("WRITE_SAME sectors: %u exceeds max_write_same_len: %u\n",
> +			sectors, cmd->se_dev->dev_attrib.max_write_same_len);
> +		return TCM_INVALID_CDB_FIELD;
> +	}

<SNIP>

Forgot to remove this now redundant check for max_write_same_len.

Dropping now..