Issue with discard with NVME and Infinibox Storage

[Keith Busch]

On Mon, Apr 03, 2023 at 01:35:22PM -0400, Laurence Oberman wrote:
> Hello Ming and Christoph
> 
> Issue with Infinibox storage
> ----------------------------
> Really discovered 2 issues here 
> 
> Issue 1
> Kernels 5.15 to 5.18 inclusive recognize the discard support on the
> Infinibox device but they fail in the nvme_setup_discard function call

This first i ssue should be fixed with this commit:

  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=37f0dc2ec78af0c3f35dd05578763de059f6fe77

> Issue 2
> Trying to narrow this down.
> 5.19 and higher (6.3 included), no longer support discard on the
> Infinibox device and log this message so I cannot run the test for the
> discard issue
> 
> [   35.989809] nvme nvme1: new ctrl: NQN "nqn.2020-
> 01.com.infinidat:36000-subsystem-696", addr 192.168.1.2:4420
> [   64.810437] XFS (nvme1n1): mounting with "discard" option, but the
> device does not support discard
> [   64.812298] XFS (nvme1n1): Mounting V5 Filesystem 6763a33f-18cc-
> 4a26-894b-8b0f8d79a98a
> 
> I then bisected between 5.18 and 5.19 to this commit
> 
> 1a86924e4f464757546d7f7bdc469be237918395 is the first bad commit


> commit 1a86924e4f464757546d7f7bdc469be237918395
> Author: Tom Yan <tom.ty89 at gmail.com>
> Date:   Fri Apr 29 12:52:43 2022 +0800
> 
>     nvme: fix interpretation of DMRSL
>     
>     DMRSLl is in the unit of logical blocks, while max_discard_sectors
> is
>     in the unit of "linux sector".
>     
>     Signed-off-by: Tom Yan <tom.ty89 at gmail.com>
>     Signed-off-by: Christoph Hellwig <hch at lst.de>
> 
>  drivers/nvme/host/core.c | 6 ++++--
>  drivers/nvme/host/nvme.h | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> 
> Note that Infindat mentioned this in our case they logged with us
> They say they fully adhere to TP4040 MDTS.
> Towards NVMe-oF 2.0 specification, TP4040  - Max Data Transfer for non-
> IO Commands (MDTS) was released with additional fields to control these
> parameters.
> These parameters are supported in kernel versions 5.15 and above.  ****
> 
> Our storage target will reply with 0 for bit 2 of the ONCS, indicating
> UNMAP is supported based on the DMRL, DMRSL, and DMSL values. 
> (older kernels will interpret these values as UNMAP NOT SUPPORTED)
> 
> 
> Let me know your thoughts please. for both issues

The commit you found unconditionally sets the discard queue limit to the
reported DMRSL, so it sounds like your target is reporting DMRSL as '0'. Prior
to that commit, we'd use that value only if it was non-zero. I hope that helps.