Re: [PATCH v1 rdma-next 2/4] RDMA/nldev: add provider-specific resource tracking

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

 



On Sat, Mar 24, 2018 at 02:42:15PM -0500, Steve Wise wrote:
> 
> > 
> > On Fri, Mar 23, 2018 at 09:49:39AM -0700, Steve Wise wrote:
> > > Each provider can register a "fill entry" function with the restrack
> core.
> > > This function will be called when filling out a resource, allowing the
> > > provider to add provider-specific details.  The details consist of a
> > > table of nested attributes, that are in the form of "key, value" tuple.
> > > The key nlattr must be strings, and the value nlattr can be one of
> > > provider attributes that are generic, but typed, allowing the nlmessage
> > > to ve validated.  Currently the types include string, s32, u32, x32,
> s64,
> > > u64, and x64. The inclusion of x, s, and u variants for numeric
> attributes
> > > allows the user tool to print the number in the desired format.
> > >
> > > More attrs can be defined as they become needed by providers.
> > >
> > > Signed-off-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> > >  drivers/infiniband/core/nldev.c  | 39
> > +++++++++++++++++++++++++++++++++++++++
> > >  include/rdma/restrack.h          | 10 ++++++++++
> > >  include/uapi/rdma/rdma_netlink.h | 16 ++++++++++++++++
> > >  3 files changed, 65 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/core/nldev.c
> > b/drivers/infiniband/core/nldev.c
> > > index 884843e..8346ede 100644
> > > +++ b/drivers/infiniband/core/nldev.c
> > > @@ -95,8 +95,27 @@
> > >  	[RDMA_NLDEV_ATTR_RES_PD_ENTRY]		= { .type =
> > NLA_NESTED },
> > >  	[RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY]	= { .type = NLA_U32 },
> > >  	[RDMA_NLDEV_ATTR_RES_UNSAFE_GLOBAL_RKEY] = { .type =
> > NLA_U32 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER]		= { .type =
> > NLA_NESTED },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_ENTRY]	= { .type =
> > NLA_NESTED },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_STRING]	= { .type =
> > NLA_NUL_STRING,
> > > +				    .len =
> > RDMA_NLDEV_ATTR_PROVIDER_STRLEN },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_D32]		= { .type = NLA_S32 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_U32]		= { .type = NLA_U32 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_X32]		= { .type = NLA_U32 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_D64]		= { .type = NLA_S64 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_U64]		= { .type = NLA_U64 },
> > > +	[RDMA_NLDEV_ATTR_PROVIDER_X64]		= { .type = NLA_U64 },
> > >  };
> > >
> > > +static int provider_fill_res_entry(struct rdma_restrack_root *resroot,
> > > +				   struct sk_buff *msg,
> > > +				   struct netlink_callback *cb,
> > > +				   struct rdma_restrack_entry *res)
> > > +{
> > > +	return resroot->fill_res_entry ?
> > > +		resroot->fill_res_entry(msg, cb, res) : 0;
> > > +}
> > 
> > Please add "fill_res_entry = NULL" line into rdma_restrack_init()
> > despite kzalloc usage in ib_alloc_device().
> 
> 
> Will do.
> 
> > 
> > And I afraid that we didn't settle the PROVIDER_*64 thing.
> > 
> 
> I didn't agree that your proposal was simpler, or even avoided the issues
> you said were problems with the self-describing-print-formt attributes.  I
> asked if anyone else had an opinion.  Nobody replied.  So I chose to keep
> the attributes as they are.  Are you NAKing this? 
> 
> Jason and Doug, do you have an opinion either way?  Our discussion of this
> can be found here:
> 
> https://www.spinics.net/lists/linux-rdma/msg62198.html

I'm still not sure what you are two are arguing about..

Is it the inclusion of 'X'? That does seem weird.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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