Issue with discard with NVME and Infinibox Storage

[Laurence Oberman]

On Mon, 2023-04-03 at 14:18 -0400, Laurence Oberman wrote:
> On Mon, 2023-04-03 at 12:00 -0600, Keith Busch wrote:
> > 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.
> > 
> 
> 
> 
> Hello Keith,
> Many Thanks as always
> I will inform Infinidat and have them figure this out.
> 
> Regards
> Laurence


Hi Keith 
Closing the loop

The challenge was the other issue was masking me testing the fix.

I reverted that commit (1a86924e4f464757546d7f7bdc469be237918395) and
now the original test passes so we are good here.

Linux localhost.localdomain 6.3.0-rc4+ #13 SMP PREEMPT_DYNAMIC Mon Apr
3 13:40:21 EDT 2023 x86_64 x86_64 x86_64 GNU/Linux

[   57.090328] nvme nvme1: new ctrl: NQN "nqn.2020-
01.com.infinidat:36000-subsystem-696", addr 192.168.1.2:4420
[   61.441213] XFS (nvme1n1): Mounting V5 Filesystem 6763a33f-18cc-
4a26-894b-8b0f8d79a98a
[   64.627670] XFS (nvme1n1): Ending clean mount
[   64.665657] xfs filesystem being mounted at /data supports
timestamps until 2038 (0x7fffffff)

Then running the fio test passes with no issues so confirming the fix
you called out (Ming had mentioned this too) resolves the original
issue.


Infinidat will have to fix their DMRSL issue

Thanks folks

Regards as always
Laurence Oberman