linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Bob Pearson <rpearsonhpe@gmail.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: Re: blktest/rxe almost working
Date: Sat, 4 Sep 2021 19:30:56 -0300	[thread overview]
Message-ID: <20210904223056.GC2505917@nvidia.com> (raw)
In-Reply-To: <711c089d-ce66-63e8-4d80-0bd19f22607c@acm.org>

On Fri, Sep 03, 2021 at 04:13:22PM -0700, Bart Van Assche wrote:
> On 9/3/21 3:18 PM, Bob Pearson wrote:
> > On 9/2/21 6:38 PM, Jason Gunthorpe wrote:
> > > On Thu, Sep 02, 2021 at 04:41:15PM -0500, Bob Pearson wrote:
> > > > Now that for-next is on 5.14.0-rc6+ blktest srp/002 is very close to
> > > > working for rxe but there is still one error. After adding MW
> > > > support I added a test to local invalidate to check and see if the
> > > > l/rkey matched the key actually contained in the MR/MW when local
> > > > invalidate is called. This is failing for srp/002 with the key
> > > > portion of the rkey off by one. Looking at ib_srp.c I see code that
> > > > does in fact increment the rkey by one and also has code that posts
> > > > a local invalidate. This was never checked before and is now failing
> > > > to match. If I mask off the key portion in the test the whole test
> > > > case passes so the other problems appear to have been fixed. If the
> > > > increment and invalidate are out of sync this could result in the
> > > > error. I suspect this may be a bug in srp. Worst case I can remove
> > > > this test but I would rather not.
> > > 
> > > I didn't check the spec, but since SRP works with HW devices I wonder
> > > if invalidation is supposed to ignore the variant bits in the mkey?
> > 
> > I am a little worried. srp is pretty complex but roughly it looks like it maintains a pool of
> > MRs which it recycles. Each time it reuses the MR it increments the key portion of the rkey. Before
> > that it uses local invalidate WRs to invalidate the MRs presumably to prevent stray accesses
> > to the old version of the MR from e.g. replicated packets. It posts these WRs to a send queue but I
> > don't see where it closes the loop by waiting for a WC so there may be a race between the invalidate
> > and the subsequent map_sg call. The invalidate marks the MR as not usable so this must all happen
> > before the MR is turned on again.
> 
> Hi Bob,
> 
> If there would be any code in the SRP driver that is not compliant with the
> IBTA specification then I can fix it.
> 
> Regarding the invalidate work requests submitted by the ib_srp driver: these
> are submitted before srp_fr_pool_put() is called. A new registration request
> is submitted after srp_fr_pool_get() succeeds. There is one MR pool per RDMA
> channel and there is one QP per RDMA channel. In other words,
> (re)registration requests are submitted to the same QP as unregistration
> requests after local invalidate requests. I think the IBTA requires does not
> allow to reorder a local invalidate followed by a fast registration request.

Right

Jason

  reply	other threads:[~2021-09-04 22:31 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 21:41 Bob Pearson
2021-09-02 23:38 ` Jason Gunthorpe
2021-09-03 22:18   ` Bob Pearson
2021-09-03 23:13     ` Bart Van Assche
2021-09-04 22:30       ` Jason Gunthorpe [this message]
2021-09-05 18:02         ` Bob Pearson
2021-09-07 12:01           ` Jason Gunthorpe
2021-09-07 16:35             ` Bob Pearson
2021-09-07 16:39               ` Jason Gunthorpe
2021-09-07 16:47                 ` Bob Pearson

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=20210904223056.GC2505917@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=bvanassche@acm.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=rpearsonhpe@gmail.com \
    --subject='Re: blktest/rxe almost working' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).