Hey HCH & Jens,
Is this already queued up for v4.13 to address the missing LBPRZ feature
bit..?
If not, I'll happy to take it via target-pending along with the
following to re-enable it via max_write_zeroes_sectors.
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index d2f089c..e7caf78 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct se_dev_attrib *attrib,
attrib->unmap_granularity = q->limits.discard_granularity / block_size;
attrib->unmap_granularity_alignment = q->limits.discard_alignment /
block_size;
- attrib->unmap_zeroes_data = 0;
+ attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
return true;
}
EXPORT_SYMBOL(target_configure_unmap_from_queue);
Any objections..?
On Tue, 2017-04-11 at 22:30 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2017-04-10 at 18:08 +0200, Christoph Hellwig wrote:
> > Use the pscsi driver to support arbitrary command passthrough
> > instead.
> >
>
> The people who are actively using iblock_execute_write_same_direct() are
> doing so in the context of ESX VAAI BlockZero, together with
> EXTENDED_COPY and COMPARE_AND_WRITE primitives. Just using PSCSI is not
> an option for them.
>
> In practice though I've not seen any users of IBLOCK WRITE_SAME for
> anything other than VAAI BlockZero, so just using blkdev_issue_zeroout()
> when available, and falling back to iblock_execute_write_same() if the
> WRITE_SAME buffer contains anything other than zeros should be OK.
>
> How about something like the following below..?
>
> This would bring parity to how blkdev_issue_write_same() works atm wrt
> to synchronous bio completions. However, most folks with a raw
> make_request or blk-mq backend driver that supports multiple GB/sec of
> zero bandwidth end up changing IBLOCK to support asynchronous
> REQ_WRITE_SAME completions anyways.
>
> I'd be happy to add support for that using __blkdev_issue_zeroout() once
> the basic conversion is in place.
>
> From ff74012eaff38f9fa0d74aca60507b9964f484ce Mon Sep 17 00:00:00 2001
> From: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> Date: Tue, 11 Apr 2017 22:21:47 -0700
> Subject: [PATCH] target/iblock: Convert WRITE_SAME to blkdev_issue_zeroout
>
> Signed-off-by: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> ---
> drivers/target/target_core_iblock.c | 44 +++++++++++++++++++++++--------------
> 1 file changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
> index d316ed5..5bfde20 100644
> --- a/drivers/target/target_core_iblock.c
> +++ b/drivers/target/target_core_iblock.c
> @@ -86,6 +86,7 @@ static int iblock_configure_device(struct se_device *dev)
> struct block_device *bd = NULL;
> struct blk_integrity *bi;
> fmode_t mode;
> + unsigned int max_write_zeroes_sectors;
> int ret = -ENOMEM;
>
> if (!(ib_dev->ibd_flags & IBDF_HAS_UDEV_PATH)) {
> @@ -129,7 +130,11 @@ static int iblock_configure_device(struct se_device *dev)
> * Enable write same emulation for IBLOCK and use 0xFFFF as
> * the smaller WRITE_SAME(10) only has a two-byte block count.
> */
> - dev->dev_attrib.max_write_same_len = 0xFFFF;
> + max_write_zeroes_sectors = bdev_write_zeroes_sectors(bd);
> + if (max_write_zeroes_sectors)
> + dev->dev_attrib.max_write_same_len = max_write_zeroes_sectors;
> + else
> + dev->dev_attrib.max_write_same_len = 0xFFFF;
>
> if (blk_queue_nonrot(q))
> dev->dev_attrib.is_nonrot = 1;
> @@ -415,28 +420,31 @@ static void iblock_end_io_flush(struct bio *bio)
> }
>
> static sense_reason_t
> -iblock_execute_write_same_direct(struct block_device *bdev, struct se_cmd *cmd)
> +iblock_execute_zero_out(struct block_device *bdev, struct se_cmd *cmd)
> {
> struct se_device *dev = cmd->se_dev;
> struct scatterlist *sg = &cmd->t_data_sg[0];
> - struct page *page = NULL;
> - int ret;
> + unsigned char *buf, zero = 0x00, *p = &zero;
> + int rc, ret;
>
> - if (sg->offset) {
> - page = alloc_page(GFP_KERNEL);
> - if (!page)
> - return TCM_OUT_OF_RESOURCES;
> - sg_copy_to_buffer(sg, cmd->t_data_nents, page_address(page),
> - dev->dev_attrib.block_size);
> - }
> + buf = kmap(sg_page(sg)) + sg->offset;
> + if (!buf)
> + return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
> + /*
> + * Fall back to block_execute_write_same() slow-path if
> + * incoming WRITE_SAME payload does not contain zeros.
> + */
> + rc = memcmp(buf, p, cmd->data_length);
> + kunmap(sg_page(sg));
> +
> + if (rc)
> + return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>
> - ret = blkdev_issue_write_same(bdev,
> + ret = blkdev_issue_zeroout(bdev,
> target_to_linux_sector(dev, cmd->t_task_lba),
> target_to_linux_sector(dev,
> sbc_get_write_same_sectors(cmd)),
> - GFP_KERNEL, page ? page : sg_page(sg));
> - if (page)
> - __free_page(page);
> + GFP_KERNEL, false);
> if (ret)
> return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
>
> @@ -472,8 +480,10 @@ static void iblock_end_io_flush(struct bio *bio)
> return TCM_INVALID_CDB_FIELD;
> }
>
> - if (bdev_write_same(bdev))
> - return iblock_execute_write_same_direct(bdev, cmd);
> + if (bdev_write_zeroes_sectors(bdev)) {
> + if (!iblock_execute_zero_out(bdev, cmd))
> + return 0;
> + }
>
> ibr = kzalloc(sizeof(struct iblock_req), GFP_KERNEL);
> if (!ibr)
