Re: [PATCHv12 1/3] rdmacg: Added rdma cgroup controller

[Matan Barak]

On 14/09/2016 10:06, Parav Pandit wrote:
> Hi Dennis,
>
> Do you know how would HFI1 driver would work along with rdma cgroup?
>
> Hi Matan, Leon, Jason,
> Apart from HFI1, is there any other concern?

I just wonder how things like RSS will work. For example, a RSS QP 
doesn't really have a queue (if I recall, it's connected to work queues 
via an indirection table). So, when a user creates such a QP, do you 
want to account it as a regular QP?
How are work queues accounted?


> Or Patch is good to go?
>
> 4.8 dates are close by (2 weeks) and there are two git trees involved
> (that might cause merge error to Linus) so if there are no issues, I
> would like to make request to Doug to consider it for 4.8 early on.
>
> Parav
>
> On Mon, Sep 12, 2016 at 10:37 AM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
> > On Sun, Sep 11, 2016 at 11:52:35AM -0600, Jason Gunthorpe wrote:
> > > On Sun, Sep 11, 2016 at 07:24:45PM +0200, Christoph Hellwig wrote:
> > > > > > > I've posted some initial work toward a) a while ago, and once we
> > > > >
> > > > > Did it get merged? Do you have a pointer?
> > > >
> > > > http://www.spinics.net/lists/linux-rdma/msg31958.html
> > >
> > > Right, I remember that. Certainly the right direction
> > >
> > > > > However, everything under verbs is not straightforward. The files in
> > > > > userspace are not copies...
> > > > >
> > > > > user:
> > > > >
> > > > > struct ibv_query_device {
> > > > >        __u32 command;
> > > > >        __u16 in_words;
> > > > >        __u16 out_words;
> > > > >        __u64 response;
> > > > >        __u64 driver_data[0];
> > > > > };
> > > > >
> > > > > kernel:
> > > > >
> > > > > struct ib_uverbs_query_device {
> > > > >         __u64 response;
> > > > >         __u64 driver_data[0];
> > > > > };
> > > >
> > > > We'll obviously need different strutures for the libibvers API
> > > > and the kernel interface in this case, and we'll need to figure out
> > > > how to properly translate them.  I think a cast, plus compile time
> > > > type checking ala BUILD_BUG_ON is the way to go.
> > >
> > > I'm not sure I follow, which would I cast?
> > >
> > > BUILD_BUG_ON(sizeof(ibv_query_device) == sizeof(ib_uverbs_cmd_hdr) +
> > >              sizeof(ib_uverbs_query_device))
> > >
> > > ?
> > >
> > > > > I'm thinking the best way forward might be to use a script and
> > > > > transform userspace into:
> > > > >
> > > > > struct ibv_query_device {
> > > > >   struct ib_uverbs_cmd_hdr hdr;
> > > > >   struct ib_uverbs_query_device cmd;
> > > > > };
> > > >
> > > > That would break the users of the interface.
> > >
> > > Sorry, I mean doing this inside rdma-plumbing. Since the change is ABI
> > > identical the modified libibverbs would still be binary compatible
> > > with all providers but not source compatible. Since all kernel
> > > supported providers are in rdma-plumbing we can add the '.cmd.' at the
> > > same time.
> > >
> > > The kernel uapi header would stay the same.
> > >
> > > > However automatically generating the user ABI from the kernel one
> > > > might still be a good idea in the long run.
> > >
> > > My preference would be to try and use the kernel headers directly.
> >
> > I thought the same, especially after realizing that they are almost
> > copy/paste from the vendor *-abi.h files.
> >
> > > Jason