All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leon Romanovsky <leon@kernel.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Yonatan Nachum <ynachum@amazon.com>,
	linux-rdma@vger.kernel.org, mrgolin@amazon.com,
	sleybo@amazon.com, matua@amazon.com, gal.pressman@linux.dev
Subject: Re: [PATCH for-next v4 0/2] RDMA/efa: Add AH cache for AH reuse
Date: Wed, 17 Jun 2026 15:28:42 +0300	[thread overview]
Message-ID: <20260617122842.GZ327369@unreal> (raw)
In-Reply-To: <20260617002145.GB3577711@nvidia.com>

On Tue, Jun 16, 2026 at 09:21:45PM -0300, Jason Gunthorpe wrote:
> On Tue, Jun 16, 2026 at 07:31:58PM +0000, Yonatan Nachum wrote:
> > A global AH cache lock would serialize all AH commands for any PD-GID
> > combination, including the ones that go to the device.
> > The per-entry mutex allows different entries to issue device commands in
> > parallel while only serializing operations on the same entry.
> > 
> > The initialized flag is needed because the entry must exist in the
> > hashtable before the device command completes, so concurrent threads
> > targeting the same PD-GID find it and wait on the per-entry mutex.
> 
> I think it looks so weird because it overloads the refcount in two
> ways.
> 
> The scheme really has two different orthogonal ideas:
>  - A kref which manages the lifetime of the entry and when the kref
>    reaches 0 the entry leaves the hash. The entry is basically just the
>    mutex and a user count. The hash holds a guarenteed singleton
>    locking point to control the HW object creation order.
> 
>  - A user count which counts how many active AH's are using the HW
>    object, and if it is non-zero then the HW object exists.
> 
> The combination of the refcount and initialized is overloading both of
> these different behaviors, along with the funky refcount logic.
> 
> But given the initialized costs as much memory in the struct as
> another refcount you may as well just replace it with a proper user
> count.
> 
> Then it is alot simpler. 
>  Global lock, do the search, get the kref, unlock and return.
>  Local lock, check the usercount == 0 and allocate HW object, incr, unlock.
> 
>  Local lock, check the usercount == 1 and dealloc the HW obhect, decr,
>  unlock. put kref.
> 
> Ideally the kref put would only grab the global lock when the refcount
> is 0, but you have to be able to tolerate multiple 0 kref things in
> the hashtable for that to work.

Jason,

I would suggest that EFA start with the basics. Their command submission
path has spinlocks, so their claim of “performance degradation” should
be viewed with an appropriate degree of skepticism.

Thanks

> 
> Jason
> 
> 
> 
> > 
> > I am open to simplifying to a single globlal lock if you prefer, but it
> > comes at the performance cost of serializing all AH commands.
> > 

      reply	other threads:[~2026-06-17 12:28 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-08  7:16 [PATCH for-next v4 0/2] RDMA/efa: Add AH cache for AH reuse Yonatan Nachum
2026-06-08  7:16 ` [PATCH for-next v4 1/2] RDMA/efa: Add initialization of AH cache rhashtable Yonatan Nachum
2026-06-08  7:16 ` [PATCH for-next v4 2/2] RDMA/efa: Add AH cache handling on create and destroy AH Yonatan Nachum
2026-06-14  7:12 ` [PATCH for-next v4 0/2] RDMA/efa: Add AH cache for AH reuse Yonatan Nachum
2026-06-16 17:50   ` Leon Romanovsky
2026-06-16 19:31     ` Yonatan Nachum
2026-06-17  0:21       ` Jason Gunthorpe
2026-06-17 12:28         ` Leon Romanovsky [this message]

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=20260617122842.GZ327369@unreal \
    --to=leon@kernel.org \
    --cc=gal.pressman@linux.dev \
    --cc=jgg@nvidia.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=matua@amazon.com \
    --cc=mrgolin@amazon.com \
    --cc=sleybo@amazon.com \
    --cc=ynachum@amazon.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.