Received-SPF: pass (google.com: best guess record for domain of linux-nfs-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Subject: Re: [PATCH v2] xprtrdma: Fix DMA scatter-gather list mapping
 imbalance
From:   Chuck Lever <chuck.lever@oracle.com>
In-Reply-To: <1d2693b1-b37f-c611-91c3-55b567be5274@tomt.net>
Date:   Wed, 12 Feb 2020 11:05:31 -0500
Cc:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
        iommu@lists.linux-foundation.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <E4C9BADB-72EA-4F39-9837-675C7E7E2CCF@oracle.com>
References: <158151473332.515306.1111360128438553868.stgit@morisot.1015granger.net>
 <869DC73D-190E-46AB-B8F8-1A394F92AF41@oracle.com>
 <1d2693b1-b37f-c611-91c3-55b567be5274@tomt.net>
To:     Andre Tomt <andre@tomt.net>
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk


> On Feb 12, 2020, at 11:03 AM, Andre Tomt <andre@tomt.net> wrote:
>=20
> On 12.02.2020 14:48, Chuck Lever wrote:
>>> On Feb 12, 2020, at 8:43 AM, Chuck Lever <chuck.lever@oracle.com> =
wrote:
>>>=20
>>> The @nents value that was passed to ib_dma_map_sg() has to be passed
>>> to the matching ib_dma_unmap_sg() call. If ib_dma_map_sg() choses to
>>> concatenate sg entries, it will return a different nents value than
>>> it was passed.
>>>=20
>>> The bug was exposed by recent changes to the AMD IOMMU driver, which
>>> enabled sg entry concatenation.
>>>=20
>>> Looking all the way back to 4143f34e01e9 ("xprtrdma: Port to new
>>> memory registration API") and reviewing other kernel ULPs, it's not
>>> clear that the frwr_map() logic was ever correct for this case.
>>>=20
>>> Reported-by: Andre Tomt <andre@tomt.net>
>>> Suggested-by: Robin Murphy <robin.murphy@arm.com>
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> ---
>>> include/trace/events/rpcrdma.h |    6 ++++--
>>> net/sunrpc/xprtrdma/frwr_ops.c |   13 +++++++------
>>> 2 files changed, 11 insertions(+), 8 deletions(-)
>>>=20
>>> Hi Andre, here's take 2, based on the trace data you sent me.
>>> Please let me know if this one fares any better.
>>>=20
>>> Changes since v1:
>>> - Ensure the correct nents value is passed to ib_map_mr_sg
>>> - Record the mr_nents value in the MR trace points
> Verified working (with the patch correction) in my environment, with =
some quick testing (mount + some random and bulk I/O)
>=20
> client, 5.5.3 + patch + amd iommu on =3D OK
> client, 5.5.3 + patch + amd iommu off =3D OK
> client, 5.6-rc1 + patch + amd iommu on =3D OK
>=20
> server, 5.5.3 + patch + intel iommu on =3D OK

Very good! I'll submit the fix through the NFS tree ASAP, and request =
backport to v5.5 stable.

--
Chuck Lever