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
