linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20
@ 2022-06-28  2:29 Mike Christie
  2022-06-28  2:29 ` [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check Mike Christie
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

The following patches were made over Linus's tree and:

[PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

They perform cleanup to target_core_file's WRITE_SAME handling and allow
users to configure UNMAP/WRITE_SAME after we have done the initial device
addition/configuration.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check
  2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
@ 2022-06-28  2:29 ` Mike Christie
  2022-06-28  5:03   ` Christoph Hellwig
  2022-06-28  2:29 ` [PATCH 2/5] scsi: target: Add callout to configure unmap settings Mike Christie
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel
  Cc: Mike Christie

We use WSNZ=1 so if we get a WRITE_SAME with zero logical blocks we are
supposed to fail it. We do this check and failure in target_core_sbc.c
before calling into the backend, so we can remove the incorrect check in
target_core_file.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_file.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index 6c8d8b051bfd..e8440e5dd804 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -438,10 +438,6 @@ fd_execute_write_same(struct se_cmd *cmd)
 	unsigned int len = 0, i;
 	ssize_t ret;
 
-	if (!nolb) {
-		target_complete_cmd(cmd, SAM_STAT_GOOD);
-		return 0;
-	}
 	if (cmd->prot_op) {
 		pr_err("WRITE_SAME: Protection information with FILEIO"
 		       " backends not supported\n");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] scsi: target: Add callout to configure unmap settings
  2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
  2022-06-28  2:29 ` [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check Mike Christie
@ 2022-06-28  2:29 ` Mike Christie
  2022-06-28  5:03   ` Christoph Hellwig
  2022-06-28  2:29 ` [PATCH 3/5] scsi: target: Add iblock configure_unmap callout Mike Christie
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel
  Cc: Mike Christie

This patch adds a callout to configure a backend's unmap settings. This
will be used in this patchset to allow userspace to setup unmap after
the initial device setup similar to how we can setup the other attrs
post device configuration.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_device.c  | 6 ++++++
 include/target/target_core_backend.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 25f33eb25337..086ac9c9343c 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -960,6 +960,12 @@ int target_configure_device(struct se_device *dev)
 	ret = dev->transport->configure_device(dev);
 	if (ret)
 		goto out_free_index;
+
+	if (dev->transport->configure_unmap &&
+	    dev->transport->configure_unmap(dev)) {
+		pr_debug("Discard support available, but disabled by default.\n");
+	}
+
 	/*
 	 * XXX: there is not much point to have two different values here..
 	 */
diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
index 773963a1e0b5..a3c193df25b3 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -37,6 +37,7 @@ struct target_backend_ops {
 	struct se_dev_plug *(*plug_device)(struct se_device *se_dev);
 	void (*unplug_device)(struct se_dev_plug *se_plug);
 
+	bool (*configure_unmap)(struct se_device *se_dev);
 	ssize_t (*set_configfs_dev_params)(struct se_device *,
 					   const char *, ssize_t);
 	ssize_t (*show_configfs_dev_params)(struct se_device *, char *);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/5] scsi: target: Add iblock configure_unmap callout
  2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
  2022-06-28  2:29 ` [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check Mike Christie
  2022-06-28  2:29 ` [PATCH 2/5] scsi: target: Add callout to configure unmap settings Mike Christie
@ 2022-06-28  2:29 ` Mike Christie
  2022-06-28  5:03   ` Christoph Hellwig
  2022-06-28  2:29 ` [PATCH 4/5] scsi: target: Add file " Mike Christie
  2022-06-28  2:29 ` [PATCH 5/5] scsi: target: Detect unmap support post configuration Mike Christie
  4 siblings, 1 reply; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel
  Cc: Mike Christie

Move iblock's unmap setup code to a configure_unmap callout.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_iblock.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 1ed9381751e6..7bef97097b78 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -76,6 +76,14 @@ static struct se_device *iblock_alloc_device(struct se_hba *hba, const char *nam
 	return NULL;
 }
 
+static bool iblock_configure_unmap(struct se_device *dev)
+{
+	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
+
+	return target_configure_unmap_from_queue(&dev->dev_attrib,
+						 ib_dev->ibd_bd);
+}
+
 static int iblock_configure_device(struct se_device *dev)
 {
 	struct iblock_dev *ib_dev = IBLOCK_DEV(dev);
@@ -119,10 +127,6 @@ static int iblock_configure_device(struct se_device *dev)
 	dev->dev_attrib.hw_max_sectors = queue_max_hw_sectors(q);
 	dev->dev_attrib.hw_queue_depth = q->nr_requests;
 
-	if (target_configure_unmap_from_queue(&dev->dev_attrib, bd))
-		pr_debug("IBLOCK: BLOCK Discard support available,"
-			 " disabled by default\n");
-
 	/*
 	 * Enable write same emulation for IBLOCK and use 0xFFFF as
 	 * the smaller WRITE_SAME(10) only has a two-byte block count.
@@ -903,6 +907,7 @@ static const struct target_backend_ops iblock_ops = {
 	.configure_device	= iblock_configure_device,
 	.destroy_device		= iblock_destroy_device,
 	.free_device		= iblock_free_device,
+	.configure_unmap	= iblock_configure_unmap,
 	.plug_device		= iblock_plug_device,
 	.unplug_device		= iblock_unplug_device,
 	.parse_cdb		= iblock_parse_cdb,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] scsi: target: Add file configure_unmap callout
  2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
                   ` (2 preceding siblings ...)
  2022-06-28  2:29 ` [PATCH 3/5] scsi: target: Add iblock configure_unmap callout Mike Christie
@ 2022-06-28  2:29 ` Mike Christie
  2022-06-28  5:04   ` Christoph Hellwig
  2022-06-28  2:29 ` [PATCH 5/5] scsi: target: Detect unmap support post configuration Mike Christie
  4 siblings, 1 reply; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel
  Cc: Mike Christie

Move file's unmap setup code to a configure_unmap callout.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_file.c | 38 +++++++++++++++++++------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index e8440e5dd804..08212eadee28 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -86,6 +86,29 @@ static struct se_device *fd_alloc_device(struct se_hba *hba, const char *name)
 	return &fd_dev->dev;
 }
 
+static bool fd_configure_unmap(struct se_device *dev)
+{
+	struct file *file = FD_DEV(dev)->fd_file;
+	struct inode *inode = file->f_mapping->host;
+
+	if (S_ISBLK(inode->i_mode)) {
+		return target_configure_unmap_from_queue(&dev->dev_attrib,
+							 I_BDEV(inode));
+	} else {
+		/*
+		 * Limit UNMAP emulation to 8k Number of LBAs (NoLB)
+		 */
+		dev->dev_attrib.max_unmap_lba_count = 0x2000;
+		/*
+		 * Currently hardcoded to 1 in Linux/SCSI code..
+		 */
+		dev->dev_attrib.max_unmap_block_desc_count = 1;
+		dev->dev_attrib.unmap_granularity = 1;
+		dev->dev_attrib.unmap_granularity_alignment = 0;
+		return true;
+	}
+}
+
 static int fd_configure_device(struct se_device *dev)
 {
 	struct fd_dev *fd_dev = FD_DEV(dev);
@@ -149,10 +172,6 @@ static int fd_configure_device(struct se_device *dev)
 			" block_device blocks: %llu logical_block_size: %d\n",
 			dev_size, div_u64(dev_size, fd_dev->fd_block_size),
 			fd_dev->fd_block_size);
-
-		if (target_configure_unmap_from_queue(&dev->dev_attrib, bdev))
-			pr_debug("IFILE: BLOCK Discard support available,"
-				 " disabled by default\n");
 		/*
 		 * Enable write same emulation for IBLOCK and use 0xFFFF as
 		 * the smaller WRITE_SAME(10) only has a two-byte block count.
@@ -170,16 +189,6 @@ static int fd_configure_device(struct se_device *dev)
 		}
 
 		fd_dev->fd_block_size = FD_BLOCKSIZE;
-		/*
-		 * Limit UNMAP emulation to 8k Number of LBAs (NoLB)
-		 */
-		dev->dev_attrib.max_unmap_lba_count = 0x2000;
-		/*
-		 * Currently hardcoded to 1 in Linux/SCSI code..
-		 */
-		dev->dev_attrib.max_unmap_block_desc_count = 1;
-		dev->dev_attrib.unmap_granularity = 1;
-		dev->dev_attrib.unmap_granularity_alignment = 0;
 
 		/*
 		 * Limit WRITE_SAME w/ UNMAP=0 emulation to 8k Number of LBAs (NoLB)
@@ -923,6 +932,7 @@ static const struct target_backend_ops fileio_ops = {
 	.configure_device	= fd_configure_device,
 	.destroy_device		= fd_destroy_device,
 	.free_device		= fd_free_device,
+	.configure_unmap	= fd_configure_unmap,
 	.parse_cdb		= fd_parse_cdb,
 	.set_configfs_dev_params = fd_set_configfs_dev_params,
 	.show_configfs_dev_params = fd_show_configfs_dev_params,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] scsi: target: Detect unmap support post configuration
  2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
                   ` (3 preceding siblings ...)
  2022-06-28  2:29 ` [PATCH 4/5] scsi: target: Add file " Mike Christie
@ 2022-06-28  2:29 ` Mike Christie
  2022-06-28  5:06   ` Christoph Hellwig
  4 siblings, 1 reply; 11+ messages in thread
From: Mike Christie @ 2022-06-28  2:29 UTC (permalink / raw)
  To: hch, martin.petersen, james.bottomley, linux-scsi, target-devel
  Cc: Mike Christie

On our backend we can do something similar to LIO where we can enable and
disable unmap support on the fly. In the scsi/block layer we can detect
this by just doing a rescan. However, LIO cannot detect this change
because we only check during the initial configuration. This patch
allows unmap detection to also happen when the user tries to turn it on.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_configfs.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index bbcbbfa72b07..f28d3c6dab98 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -732,6 +732,7 @@ static ssize_t emulate_tpu_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	struct se_dev_attrib *da = to_attrib(item);
+	struct se_device *dev = da->da_dev;
 	bool flag;
 	int ret;
 
@@ -744,8 +745,11 @@ static ssize_t emulate_tpu_store(struct config_item *item,
 	 * Discard supported is detected iblock_create_virtdevice().
 	 */
 	if (flag && !da->max_unmap_block_desc_count) {
-		pr_err("Generic Block Discard not supported\n");
-		return -ENOSYS;
+		if (!dev->transport->configure_unmap ||
+		    !dev->transport->configure_unmap(dev)) {
+			pr_err("Generic Block Discard not supported\n");
+			return -ENOSYS;
+		}
 	}
 
 	da->emulate_tpu = flag;
@@ -758,6 +762,7 @@ static ssize_t emulate_tpws_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	struct se_dev_attrib *da = to_attrib(item);
+	struct se_device *dev = da->da_dev;
 	bool flag;
 	int ret;
 
@@ -770,8 +775,11 @@ static ssize_t emulate_tpws_store(struct config_item *item,
 	 * Discard supported is detected iblock_create_virtdevice().
 	 */
 	if (flag && !da->max_unmap_block_desc_count) {
-		pr_err("Generic Block Discard not supported\n");
-		return -ENOSYS;
+		if (!dev->transport->configure_unmap ||
+		    !dev->transport->configure_unmap(dev)) {
+			pr_err("Generic Block Discard not supported\n");
+			return -ENOSYS;
+		}
 	}
 
 	da->emulate_tpws = flag;
@@ -964,6 +972,7 @@ static ssize_t unmap_zeroes_data_store(struct config_item *item,
 		const char *page, size_t count)
 {
 	struct se_dev_attrib *da = to_attrib(item);
+	struct se_device *dev = da->da_dev;
 	bool flag;
 	int ret;
 
@@ -982,10 +991,12 @@ static ssize_t unmap_zeroes_data_store(struct config_item *item,
 	 * Discard supported is detected iblock_configure_device().
 	 */
 	if (flag && !da->max_unmap_block_desc_count) {
-		pr_err("dev[%p]: Thin Provisioning LBPRZ will not be set"
-		       " because max_unmap_block_desc_count is zero\n",
-		       da->da_dev);
-		return -ENOSYS;
+		if (!dev->transport->configure_unmap ||
+		    !dev->transport->configure_unmap(dev)) {
+			pr_err("dev[%p]: Thin Provisioning LBPRZ will not be set because max_unmap_block_desc_count is zero\n",
+			      da->da_dev);
+			return -ENOSYS;
+		}
 	}
 	da->unmap_zeroes_data = flag;
 	pr_debug("dev[%p]: SE Device Thin Provisioning LBPRZ bit: %d\n",
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check
  2022-06-28  2:29 ` [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check Mike Christie
@ 2022-06-28  5:03   ` Christoph Hellwig
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2022-06-28  5:03 UTC (permalink / raw)
  To: Mike Christie
  Cc: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

On Mon, Jun 27, 2022 at 09:29:49PM -0500, Mike Christie wrote:
> We use WSNZ=1 so if we get a WRITE_SAME with zero logical blocks we are
> supposed to fail it. We do this check and failure in target_core_sbc.c
> before calling into the backend, so we can remove the incorrect check in
> target_core_file.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/5] scsi: target: Add callout to configure unmap settings
  2022-06-28  2:29 ` [PATCH 2/5] scsi: target: Add callout to configure unmap settings Mike Christie
@ 2022-06-28  5:03   ` Christoph Hellwig
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2022-06-28  5:03 UTC (permalink / raw)
  To: Mike Christie
  Cc: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

On Mon, Jun 27, 2022 at 09:29:50PM -0500, Mike Christie wrote:
> This patch adds a callout to configure a backend's unmap settings. This
> will be used in this patchset to allow userspace to setup unmap after
> the initial device setup similar to how we can setup the other attrs
> post device configuration.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/5] scsi: target: Add iblock configure_unmap callout
  2022-06-28  2:29 ` [PATCH 3/5] scsi: target: Add iblock configure_unmap callout Mike Christie
@ 2022-06-28  5:03   ` Christoph Hellwig
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2022-06-28  5:03 UTC (permalink / raw)
  To: Mike Christie
  Cc: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/5] scsi: target: Add file configure_unmap callout
  2022-06-28  2:29 ` [PATCH 4/5] scsi: target: Add file " Mike Christie
@ 2022-06-28  5:04   ` Christoph Hellwig
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2022-06-28  5:04 UTC (permalink / raw)
  To: Mike Christie
  Cc: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

On Mon, Jun 27, 2022 at 09:29:52PM -0500, Mike Christie wrote:
> +static bool fd_configure_unmap(struct se_device *dev)
> +{
> +	struct file *file = FD_DEV(dev)->fd_file;
> +	struct inode *inode = file->f_mapping->host;
> +
> +	if (S_ISBLK(inode->i_mode)) {
> +		return target_configure_unmap_from_queue(&dev->dev_attrib,
> +							 I_BDEV(inode));
> +	} else {

No need for the else here.

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 5/5] scsi: target: Detect unmap support post configuration
  2022-06-28  2:29 ` [PATCH 5/5] scsi: target: Detect unmap support post configuration Mike Christie
@ 2022-06-28  5:06   ` Christoph Hellwig
  0 siblings, 0 replies; 11+ messages in thread
From: Christoph Hellwig @ 2022-06-28  5:06 UTC (permalink / raw)
  To: Mike Christie
  Cc: hch, martin.petersen, james.bottomley, linux-scsi, target-devel

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-06-28  5:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-28  2:29 [PATCH 0/5] target: UNMAP/WRITE_SAME features/cleanups for 5.20 Mike Christie
2022-06-28  2:29 ` [PATCH 1/5] scsi: target: Remove incorrect zero blocks WRITE_SAME check Mike Christie
2022-06-28  5:03   ` Christoph Hellwig
2022-06-28  2:29 ` [PATCH 2/5] scsi: target: Add callout to configure unmap settings Mike Christie
2022-06-28  5:03   ` Christoph Hellwig
2022-06-28  2:29 ` [PATCH 3/5] scsi: target: Add iblock configure_unmap callout Mike Christie
2022-06-28  5:03   ` Christoph Hellwig
2022-06-28  2:29 ` [PATCH 4/5] scsi: target: Add file " Mike Christie
2022-06-28  5:04   ` Christoph Hellwig
2022-06-28  2:29 ` [PATCH 5/5] scsi: target: Detect unmap support post configuration Mike Christie
2022-06-28  5:06   ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).