Re: [PATCH rdma-next v3 02/20] RDMA/bnxt_re: Initialize ib_device_ops struct

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 06, 2018 at 10:03:33PM +0200, Kamal Heib wrote:
> On Tue, Nov 06, 2018 at 08:51:00AM -0700, Jason Gunthorpe wrote:
> > On Mon, Nov 05, 2018 at 01:35:10PM +0200, Kamal Heib wrote:
> > > Initialize ib_device_ops with the supported operations using
> > > ib_set_device_ops().
> > > 
> > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > >  drivers/infiniband/hw/bnxt_re/main.c | 107 ++++++++++++++++++-----------------
> > >  1 file changed, 56 insertions(+), 51 deletions(-)
> > > 
> > > diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> > > index cf2282654210..6e7b3eb75fce 100644
> > > +++ b/drivers/infiniband/hw/bnxt_re/main.c
> > > @@ -568,6 +568,61 @@ static void bnxt_re_unregister_ib(struct bnxt_re_dev *rdev)
> > >  	ib_unregister_device(&rdev->ibdev);
> > >  }
> > >  
> > > +static const struct ib_device_ops bnxt_re_dev_ops = {
> > > +	/* Device operations */
> > > +	.query_device = bnxt_re_query_device,
> > > +	.modify_device = bnxt_re_modify_device,
> > > +	.get_dev_fw_str = bnxt_re_query_fw_str,
> > > +	/* Port operations */
> > > +	.query_port = bnxt_re_query_port,
> > > +	.get_port_immutable = bnxt_re_get_port_immutable,
> > > +	.query_pkey = bnxt_re_query_pkey,
> > > +	.get_netdev = bnxt_re_get_netdev,
> > > +	.get_link_layer = bnxt_re_get_link_layer,
> > > +	/* GID operations */
> > > +	.add_gid = bnxt_re_add_gid,
> > > +	.del_gid = bnxt_re_del_gid,
> > > +	/* PD operations */
> > > +	.alloc_pd = bnxt_re_alloc_pd,
> > > +	.dealloc_pd = bnxt_re_dealloc_pd,
> > > +	/* AH operations */
> > > +	.create_ah = bnxt_re_create_ah,
> > > +	.modify_ah = bnxt_re_modify_ah,
> > > +	.query_ah = bnxt_re_query_ah,
> > > +	.destroy_ah = bnxt_re_destroy_ah,
> > > +	/* SRQ operations */
> > > +	.create_srq = bnxt_re_create_srq,
> > > +	.modify_srq = bnxt_re_modify_srq,
> > > +	.query_srq = bnxt_re_query_srq,
> > > +	.destroy_srq = bnxt_re_destroy_srq,
> > > +	.post_srq_recv = bnxt_re_post_srq_recv,
> > > +	/* QP operations */
> > > +	.create_qp = bnxt_re_create_qp,
> > > +	.modify_qp = bnxt_re_modify_qp,
> > > +	.query_qp = bnxt_re_query_qp,
> > > +	.destroy_qp = bnxt_re_destroy_qp,
> > > +	.post_send = bnxt_re_post_send,
> > > +	.post_recv = bnxt_re_post_recv,
> > > +	/* CQ operations */
> > > +	.create_cq = bnxt_re_create_cq,
> > > +	.destroy_cq = bnxt_re_destroy_cq,
> > > +	.poll_cq = bnxt_re_poll_cq,
> > > +	.req_notify_cq = bnxt_re_req_notify_cq,
> > > +	/* MR operations */
> > > +	.get_dma_mr = bnxt_re_get_dma_mr,
> > > +	.dereg_mr = bnxt_re_dereg_mr,
> > > +	.alloc_mr = bnxt_re_alloc_mr,
> > > +	.map_mr_sg = bnxt_re_map_mr_sg,
> > > +	.reg_user_mr = bnxt_re_reg_user_mr,
> > > +	/* Ucontext operations */
> > > +	.alloc_ucontext = bnxt_re_alloc_ucontext,
> > > +	.dealloc_ucontext = bnxt_re_dealloc_ucontext,
> > > +	.mmap = bnxt_re_mmap,
> > > +	/* Stats operations */
> > > +	.get_hw_stats = bnxt_re_ib_get_hw_stats,
> > > +	.alloc_hw_stats = bnxt_re_ib_alloc_hw_stats,
> > > +};
> > 
> > I think the comment last time was keep sorted, why make this into
> > something even harder to maintain properly? The comments here add no
> > value at all.
> > 
> > Don't like.
> > 
> > Jason
> 
> I see, so you prefer to sort the operation alphabetically within the providers?
> 
> Are you fine with the functionality sort that was done within the ib_device_ops
> struct (patch #1)?

I'm sort of 'meh' on it.

If you want to sort for performance it needs to be usage driven, not
something arbitary like by category.

Ie there is no reason to put infrequent control plane stuff like
destroy_qp in the same cacheline as post_send/post_recv - there are
surely better datapath things that could go in that hot cacheline,
like poll_cq for instance.

Jason



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux