Repository navigation
NAS-144298 / 27.0.0 / nvmet-tcp: report a bounded MDTS instead of "no limit" (by ixhamza) - #392
Merged
Merged
Conversation
nvmet-tcp does not implement .get_mdts, so nvmet_ctrl_mdts() falls back
to the port value (0 by default) and identify-controller advertises "no
maximum data transfer size". The initiator believes it:
nvme_init_ctrl_finish() sets max_hw_sectors to UINT_MAX.
What the initiator then issues is decided by the block layer. The
generic cap is 4 MiB (BLK_DEF_MAX_SECTORS_CAP), but a namespace that
advertises a large NOWS raises it: blk_validate_limits() takes
max_sectors from io_opt once io_opt exceeds that cap. On the array we
measured, NOWS is 65535, so io_opt is 32 MiB and the initiator emitted
32 MiB commands -- from a controller that advertised no MDTS at all.
The target cannot serve those, in one of two ways depending on the
kernel.
Since commit 4a3f00262a04 ("nvmet-tcp: bound SGL data length before
allocating command buffers"), nvmet_tcp_map_data() rejects any len above
NVMET_TCP_MAXH2CDATA (4 MiB) with NVME_SC_SGL_INVALID_DATA | DNR, so the
command fails hard and deterministically. That is the right response to
an oversized command, but the initiator had no way to avoid sending it:
nothing advertised the limit it was exceeding.
Before that commit the command reached sgl_alloc(), which for 32 MiB
needs 8192 scatterlist entries -- 256 KiB, an order-6 kmalloc. Under
fragmentation that fails, and it surfaces as NVME_SC_INTERNAL, a generic
status, so NVMe multipath does not fail the command over to another
path. A 1 MiB command needs 256 entries, 8 KiB, served from the
kmalloc-8k slab, which does not fail. That failure mode is intermittent
and load-dependent rather than deterministic; on a vendor kernel
predating the bound it cost us roughly 98 GiB of a 2 TiB image copy,
with the copy tool exiting 0.
Commit ec6d20e ("nvmet-rdma: Implement get_mdts controller op")
left the other transports untouched. Do the same for TCP, using the
same 1 MiB value, so initiators size their commands to something the
target accepts instead of discovering the limit by failing.
Signed-off-by: Alfonso Kuen <gerencia@idkmanager.com>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Link: https://lore.kernel.org/all/20260831005117.142713-1-gerencia@idkmanager.com/
(cherry picked from commit 00829b7)
Author
ixhamza
approved these changes
Oct 30, 2026
Author
|
This PR has been merged and conversations have been locked. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Since 4a3f00262a04 came in through stable with 6.12.106 and 6.18.47,
nvmet-tcprejects any command above 4 MiB while still advertisingMDTS=0. zvols report a 32 MiB optimal IO size, so Linux hosts raisemax_sectors_kbto 32 MiB on their own and their large IOs fail withEIO, which can silently lose data on ext4. This carries the upstream fix below, which has aReviewed-bybut isn't merged yet, until it lands upstream and in stable.Tested on the 25.10.8 (6.12) and master (6.18) kernels with 6.12, 6.18 and mainline hosts. Direct IO up to 32 MiB,
mkfs, anext4write checked after remount, and fio withcrc32cverify all pass, with no throughput change.Upstream Commit
nvmet-tcp does not implement .get_mdts, so nvmet_ctrl_mdts() falls back to the port value (0 by default) and identify-controller advertises "no maximum data transfer size". The initiator believes it: nvme_init_ctrl_finish() sets max_hw_sectors to UINT_MAX.
What the initiator then issues is decided by the block layer. The generic cap is 4 MiB (BLK_DEF_MAX_SECTORS_CAP), but a namespace that advertises a large NOWS raises it: blk_validate_limits() takes max_sectors from io_opt once io_opt exceeds that cap. On the array we measured, NOWS is 65535, so io_opt is 32 MiB and the initiator emitted 32 MiB commands -- from a controller that advertised no MDTS at all.
The target cannot serve those, in one of two ways depending on the kernel.
Since commit 4a3f00262a04 ("nvmet-tcp: bound SGL data length before allocating command buffers"), nvmet_tcp_map_data() rejects any len above NVMET_TCP_MAXH2CDATA (4 MiB) with NVME_SC_SGL_INVALID_DATA | DNR, so the command fails hard and deterministically. That is the right response to an oversized command, but the initiator had no way to avoid sending it: nothing advertised the limit it was exceeding.
Before that commit the command reached sgl_alloc(), which for 32 MiB needs 8192 scatterlist entries -- 256 KiB, an order-6 kmalloc. Under fragmentation that fails, and it surfaces as NVME_SC_INTERNAL, a generic status, so NVMe multipath does not fail the command over to another path. A 1 MiB command needs 256 entries, 8 KiB, served from the kmalloc-8k slab, which does not fail. That failure mode is intermittent and load-dependent rather than deterministic; on a vendor kernel predating the bound it cost us roughly 98 GiB of a 2 TiB image copy, with the copy tool exiting 0.
Commit ec6d20e ("nvmet-rdma: Implement get_mdts controller op") left the other transports untouched. Do the same for TCP, using the same 1 MiB value, so initiators size their commands to something the target accepts instead of discovering the limit by failing.
Reviewed-by: Sagi Grimberg sagi@grimberg.me
Link: https://lore.kernel.org/all/20260831005117.142713-1-gerencia@idkmanager.com/
Original PR: #390