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@xxxxxxxxx> > > 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@xxxxxxxxx> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > > > > 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