[Drbd-dev] [PATCH 25/27] block: remove the discard_zeroes_data flag

[Bart Van Assche]

On Wed, 2017-04-05 at 19:21 +0200, Christoph Hellwig wrote:
> Now that we use the proper REQ_OP_WRITE_ZEROES operation everywhere we can
> kill this hack.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen at oracle.com>
> Reviewed-by: Hannes Reinecke <hare at suse.com>
> [ ... ]
> diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
> index c754ae33bf7b..d2f089cfa9ae 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 = q->limits.discard_zeroes_data;
> +	attrib->unmap_zeroes_data = 0;
>  	return true;
>  }
>  EXPORT_SYMBOL(target_configure_unmap_from_queue);

Hello Christoph,

Sorry that I hadn't noticed this before but I think that this patch
introduces a significant performance regressions for LIO users. Before
this patch the LBPRZ flag was reported correctly to initiator systems
through the thin provisioning VPD. With this patch applied that flag
will always be reported as zero, forcing initiators to submit WRITE
commands with zeroed data buffers instead of submitting the SCSI UNMAP
command to block devices for which discard_zeroes_data was set. From
target_core_spc.c:

/* Thin Provisioning VPD */
static sense_reason_t spc_emulate_evpd_b2(struct se_cmd *cmd, unsigned char *buf)
{
	[ ... ]
	/*
	 * The unmap_zeroes_data set means that the underlying device supports
	 * REQ_DISCARD and has the discard_zeroes_data bit set. This satisfies
	 * the SBC requirements for LBPRZ, meaning that a subsequent read
	 * will return zeroes after an UNMAP or WRITE SAME (16) to an LBA
	 * See sbc4r36 6.6.4.
	 */
	if (((dev->dev_attrib.emulate_tpu != 0) ||
	     (dev->dev_attrib.emulate_tpws != 0)) &&
	     (dev->dev_attrib.unmap_zeroes_data != 0))
		buf[5] |= 0x04;
	[ ... ]
}

Please advise how to proceed.

Thanks,

Bart.