All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bob Pearson <rpearsonhpe@gmail.com>
To: Jason Gunthorpe <jgg@nvidia.com>,
	Zhu Yanjun <zyjzyj2000@gmail.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: much about ah objects in rxe
Date: Fri, 22 Apr 2022 13:32:24 -0500	[thread overview]
Message-ID: <983bec37-4765-b45e-0f73-c474976d2dfc@gmail.com> (raw)

Jason,

I am confused a little.

 - xa_alloc_xxx internally takes xa->xa_lock with a spinlock but
   has a gfp_t parameter which is normally GFP_KERNEL. So I trust them when they say
   that it releases the lock around kmalloc's by 'magic' as you say.

 - The only read side operation on the rxe pool xarrays is in rxe_pool_get_index() but
   that will be protected by a rcu_read_lock so it can't deadlock with the write
   side spinlocks regardless of type (plain, _bh, _saveirq)

 - Apparently CM is calling ib_create_ah while holding spin locks. This would
   call xa_alloc_xxx which would unlock xa_lock and call kmalloc(..., GFP_KERNEL)
   which should cause a warning for AH. You say it does not because xarray doesn't
   call might_sleep().

I am not sure how might_sleep() works. When I add might_sleep() just ahead of
xa_alloc_xxx() it does not cause warnings for CM test cases (e.g. rping.)
Another way to study this would be to test for in_atomic() but
that seems to be discouraged and may not work as assumed. It's hard to reproduce
evidence that ib_create_ah really has spinlocks held by the caller. I think it
was seen in lockdep traces but I have a hard time reading them.

 - There is a lot of effort trying to make 'deadlocks' go away. But the read side
   is going to end as up rcu_read_lock so there soon will be no deadlocks with
   rxe_pool_get_index() possible. xarrays were designed to work well with rcu
   so it would better to just go ahead and do it. Verbs objects tend to be long
   lived with lots of IO on each instance. This is a perfect use case for rcu.

I think this means there is no reason for anything but a plain spinlock in rxe_alloc
and rxe_add_to_pool.

To sum up once we have rcu enabled the only required change is to use GFP_ATOMIC
or find a way to pre-allocate for AH objects (assuming that I can convince myself
that ib_create_ah really comes with spinlocks held).

Bob

             reply	other threads:[~2022-04-22 21:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-22 18:32 Bob Pearson [this message]
2022-04-22 21:00 ` much about ah objects in rxe Jason Gunthorpe
2022-04-22 22:11   ` Bob Pearson
2022-04-23  1:18     ` Zhu Yanjun
2022-04-23  1:48       ` Bob Pearson
2022-04-23  2:35         ` Zhu Yanjun
2022-04-23  4:34           ` Bob Pearson
2022-04-23  8:02             ` Zhu Yanjun

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=983bec37-4765-b45e-0f73-c474976d2dfc@gmail.com \
    --to=rpearsonhpe@gmail.com \
    --cc=jgg@nvidia.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=zyjzyj2000@gmail.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.