Thanks for your informative explanations. Reviewed-by: Guy Levi <guyle@xxxxxxxxxxxx> :) > -----Original Message----- > From: Jason Gunthorpe > Sent: Wednesday, August 15, 2018 2:09 AM > To: Guy Levi(SW) <guyle@xxxxxxxxxxxx> > Cc: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>; Yishai Hadas <yishaih@xxxxxxxxxxxx>; Ruhl, Michael J > <michael.j.ruhl@xxxxxxxxx> > Subject: Re: [PATCH 02/10] IB/uverbs: Build the specs into a radix tree at runtime > > On Tue, Aug 14, 2018 at 06:22:54AM -0600, Guy Levi(SW) wrote: > > > > > From: Jason Gunthorpe > > > Sent: Monday, August 13, 2018 7:03 PM > > > To: Guy Levi(SW) <guyle@xxxxxxxxxxxx> > > > Cc: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>; Yishai Hadas <yishaih@xxxxxxxxxxxx>; Ruhl, > Michael J > > > <michael.j.ruhl@xxxxxxxxx> > > > Subject: Re: [PATCH 02/10] IB/uverbs: Build the specs into a radix tree at runtime > > > > > > On Mon, Aug 13, 2018 at 08:52:35AM -0600, Guy Levi(SW) wrote: > > > > Well, better late than never... > > > > SB my comments :) > > > > > > > > > From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > > > > > Sent: Friday, August 03, 2018 10:32 PM > > > > > To: linux-rdma@xxxxxxxxxxxxxxx; Leon Romanovsky <leonro@xxxxxxxxxxxx>; Guy Levi(SW) <guyle@xxxxxxxxxxxx>; Yishai > > > Hadas > > > > > <yishaih@xxxxxxxxxxxx>; Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx> > > > > > Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > > Subject: [PATCH 02/10] IB/uverbs: Build the specs into a radix tree at runtime > > > > > > > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > > > > > > > This radix tree datastructure is intended to replace the 'hash' structure > > > > > used today for parsing ioctl methods during system calls. This first > > > > > commit introduces the structure and builds it from the existing .rodata > > > > > descriptions. > > > > > > > > > > The so-called hash arrangement is actually a 5 level open coded radix tree. > > > > > This new version uses a 3 level radix tree built using the radix tree > > > > > library. > > > > > > > > > > Overall this is much less code and much easier to build as the radix tree > > > > > API allows for dynamic modification during the building. There is a small > > > > > memory penalty to pay for this, but since the radix tree is allocated on > > > > > a per device basis, a few kb of RAM seems immaterial considering the > > > > > gained simplicity. > > > > > > > > > > The radix tree is similar to the existing tree, but also has a 'attr_bkey' > > > > > + return -EINVAL; > > > > > + } else { > > > > > + obj_elm->type_attrs = obj->type_attrs; > > > > > + if (obj->type_attrs) { > > > > > > > > Can be an obj w/o type_attrs? > > > > Does the DSL let it? > > > > How would it be used and for what purpose? > > > > > > This flow is used by the DEVICE object, and other objects that cannot > > > be created. The DSL looks like this: > > > > > > DECLARE_UVERBS_GLOBAL_METHODS(UVERBS_OBJECT_DEVICE); > > > > What is the purpose of such object? Does it have methods? > > Can't be created by whom? > > In this flow the driver has a non-exist object in his spec tree... so it is actually trying to create it... > > It is a peculiar thing, but this is how 'static' methods are created. > > In this case it would be very strange to not have a method, but Matan > roughedd this in to hold the get_context call. > > The mlx5 driver uses this for some non-object devx methods. > > > > Currently we have lots of object defined with no ioctl methods because > > > the kabi is not complete. The methods exist on the write side. > > > > > > We still have to create the infrastructure though as we now require > > > the struct uverbs_obj_type to exist for all object. So yes, we must > > > inster the empty object in to the tree. > > > > AFAIU, it is expected to occur on the default tree only, right? > > Probably. > > > Maybe should be prior check if (is_driver && !obj->methods) -> > > fail.... Or it is already protected by the DSL... > > Not sure it is worth it - no methods is functional fine, just a > strange thing conceptually. > > Jason
