mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefan Metzmacher <metze@samba.org>
To: linux-rdma@vger.kernel.org
Cc: Tatyana Nikolova <tatyana.e.nikolova@intel.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>,
	Mustafa Ismail <mustafa.ismail@intel.com>,
	Shiraz Saleem <shiraz.saleem@intel.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] RDMA/irdma: always route IB_WR_REG_MR through the allocated PBL
Date: Wed, 7 Oct 2026 12:26:25 +0200	[thread overview]
Message-ID: <acccb494-988e-4b87-b40c-42421d2e31e5@samba.org> (raw)
In-Reply-To: <20261007095312.1523211-1-metze@samba.org>

Am 07.10.26 um 11:53 schrieb Stefan Metzmacher:
> irdma_alloc_mr() always allocates a PBL via irdma_get_pble(), but
> irdma_post_send()'s IB_WR_REG_MR handling only pointed the FAST_REGISTER
> WQE at that PBL (chunk_size != 0, first_pm_pbl_index) when
> iwmr->npages exceeded IRDMA_MIN_PAGES_PER_FMR (1) - leaving chunk_size
> at 0 for a single-page (npages == 1) registration, telling hardware to
> use direct/PBL-less addressing for a registration that was never
> allocated that way.
> 
> This reproduces as data corruption / an IOMMU IO_PAGE_FAULT when a
> peer later performs an RDMA WRITE/READ against such a single-page
> FRWR-registered MR, even though ib_map_mr_sg() itself reports success.
> 100% reproducible on iWARP, intermittent on RoCEv2, with fs/smb/smbdirect's
> own single-page memory registrations on an Intel E810-CQDA2 (irdma).
> 
> Intel's current out-of-tree irdma driver (2.6.96) never gates this on
> npages - it always sets chunk_size/first_pm_pbl_index based on the
> actual PBL level (PBLE_LEVEL_1 vs PBLE_LEVEL_2), matching what
> irdma_alloc_mr() actually allocated. Mirror that here instead of the
> npages-based heuristic.
> 
> Also drop the reg_addr_pa assignment. irdma_sc_mr_fast_register()
> packs reg_addr_pa into the FAST_REGISTER WQE's PBLADDR field, which
> hardware only consults when chunk_size == 0 (direct/PBL-less
> addressing); with chunk_size now always nonzero for this opcode, that
> field is never read for IB_WR_REG_MR and the assignment is dead code -
> Intel's driver never sets it for this path either (only the separate,
> non-FRWR irdma_sc_mr_reg_non_shared() path actually branches on
> chunk_size between reg_addr_pa and first_pm_pbl_index). Removing it is
> not required for correctness: *palloc->level1.addr is always a
> page-aligned address (low 12 bits zero), so it could never have
> collided with the first_pm_pbl_index bits sharing those same bit
> positions in the WQE word even if some path still read it.
> 
> This leaves one deliberate difference from irdma-2.6.96: that driver also
> ties local_fence to the WR's IB_SEND_FENCE flag
> (stag_info.local_fence = ib_wr->send_flags & IB_SEND_FENCE) rather than
> hardcoding it true for IB_WR_REG_MR. That is a separate, non-corruption
> behavioural change, so local_fence is left at true here.

I realized to late this was changed just recently in:

commit 3fb905f07ea45b31c8f67ba6e4668de46f527e65
Author:     Jacob Moroni <jmoroni@google.com>
AuthorDate: Tue Sep 1 16:00:14 2026 +0000
Commit:     Leon Romanovsky <leon@kernel.org>
CommitDate: Wed Sep 2 12:27:03 2026 -0400

     RDMA/irdma: Enforce local fence for IB_WR_REG_MR

I also noticed that these two from linux-rdma/for-next
are in conflict:

4a93466061d38f159b857aad63a9cc3cd3affadb RDMA/irdma: Remove unused post_sq arguments
c470050f2a7a7748ce35367e828885a8e658d712 RDMA/irdma: Preserve fast-registration IOVA on 32-bit

What's best way forward here, is this something that
would go via linux-rdma/for-rc or via linux-rdma/for-next?

as we're at rc6 already waiting ~3 weeks before it hits upstream
is fine, I have it in my local test kernels anyway...

Should I resubmit a v2 with the commit message clarified
regarding "RDMA/irdma: Enforce local fence for IB_WR_REG_MR"
and should I base this on top of linux-rdma/for-rc or linux-rdma/for-next?

Thanks!
metze


  reply	other threads:[~2026-10-07 10:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-07  9:53 Stefan Metzmacher
2026-10-07 10:26 ` Stefan Metzmacher [this message]
2026-10-12  6:33   ` Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acccb494-988e-4b87-b40c-42421d2e31e5@samba.org \
    --to=metze@samba.org \
    --cc=jgg@ziepe.ca \
    --cc=leon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=mustafa.ismail@intel.com \
    --cc=shiraz.saleem@intel.com \
    --cc=tatyana.e.nikolova@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®