linux-rdma.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bob Pearson <rpearsonhpe@gmail.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: zyjzyj2000@gmail.com, linux-rdma@vger.kernel.org
Subject: Re: [PATCH for-next v10 07/17] RDMA/rxe: Use kzmalloc/kfree for mca
Date: Tue, 1 Feb 2022 14:30:43 -0600	[thread overview]
Message-ID: <f01397f3-4e75-933f-2d7c-2ec0b7a757e5@gmail.com> (raw)
In-Reply-To: <20220201201452.GO1786498@nvidia.com>

On 2/1/22 14:14, Jason Gunthorpe wrote:
> On Tue, Feb 01, 2022 at 02:00:09PM -0600, Bob Pearson wrote:
> 
> 
>> as currently written the local variable has a kref obtained from the kref_get in
>> rxe_lookup_mcg or the kref_init in rxe_init_mcg if it is newly created. This ref is
>> dropped when the local variable goes out of scope. To protect the mcg when it is
>> inactive at least one more ref is required. I take an additional ref in rxe_get_mcg
>> if the mcg is created to protect the pointer in the red-black tree. This persists
>> for the lifetime of the object until it is destroyed when it is removed from the tree
>> and has the longest scope. This is enough to keep the object alive (it works fine BTW.)
>> It is also possible to take ref's representing the pointers in the list but they
>> wouldn't add anything.
> 
> I think I got it upside down, but OK this works for me. What was
> kbuild complaining about?
> 
>> On the other point. Is it standard practice to user ERRPTRs in the
>> kernel rather than return arguments? I seem to have seen both styles
>> used but it may have been my imagination. I don't have any
>> preference here but sometimes choose one or the other in parallel
>> flows to make comparable routines in the flows have similar
>> interfaces.
> 
> Always prefer errptrs.
> 
> Jason

this API is a little different than most of the verbs APIs where typically we have

obj = xx_create_obj()
	takes a reference at kref_init() and doesn't drop it on the non error path.
	returns obj.

xx_other_code_using_obj(obj)
	uses obj holding ref

xx_destroy_obj(obj)
	drops the ref to obj

here the object is a local variable in xx_mcast_attach() and nothing is returned.

For InfiniBand the create and destroy mcast group function is triggered by MADs
from some central mcast server. For RoCEv2 that doesn't happen and these APIs
don't exist.

	
a missing static declaration at line 262. If you want me to I can fix it and resend.
Do I really have to mint new version?

Bob


  reply	other threads:[~2022-02-01 20:30 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-31 22:08 [PATCH for-next v10 00/17] Move two object pools to rxe_mcast.c Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 01/17] RDMA/rxe: Move rxe_mcast_add/delete " Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 02/17] RDMA/rxe: Move rxe_mcast_attach/detach " Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 03/17] RDMA/rxe: Rename rxe_mc_grp and rxe_mc_elem Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 04/17] RDMA/rxe: Enforce IBA o10-2.2.3 Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 05/17] RDMA/rxe: Remove rxe_drop_all_macst_groups Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 06/17] RDMA/rxe: Remove qp->grp_lock and qp->grp_list Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 07/17] RDMA/rxe: Use kzmalloc/kfree for mca Bob Pearson
2022-02-01  1:03   ` kernel test robot
2022-02-01 14:53   ` Jason Gunthorpe
2022-02-01 20:00     ` Bob Pearson
2022-02-01 20:14       ` Jason Gunthorpe
2022-02-01 20:30         ` Bob Pearson [this message]
2022-02-01 18:20   ` kernel test robot
2022-01-31 22:08 ` [PATCH for-next v10 08/17] RDMA/rxe: Rename grp to mcg and mce to mca Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 09/17] RDMA/rxe: Introduce RXECB(skb) Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 10/17] RDMA/rxe: Split rxe_rcv_mcast_pkt into two phases Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 11/17] RDMA/rxe: Replace mcg locks by rxe->mcg_lock Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 12/17] RDMA/rxe: Replace pool key by rxe->mcg_tree Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 13/17] RDMA/rxe: Remove key'ed object support Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 14/17] RDMA/rxe: Remove mcg from rxe pools Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 15/17] RDMA/rxe: Add code to cleanup mcast memory Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 16/17] RDMA/rxe: Add comments to rxe_mcast.c Bob Pearson
2022-01-31 22:08 ` [PATCH for-next v10 17/17] RDMA/rxe: Finish cleanup of rxe_mcast.c Bob Pearson
2022-02-01 14:36 ` [PATCH for-next v10 00/17] Move two object pools to rxe_mcast.c Jason Gunthorpe

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=f01397f3-4e75-933f-2d7c-2ec0b7a757e5@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 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).