Re: [PATCH, RFC] rdma: split struct ib_send_wr

Re: [PATCH, RFC] rdma: split struct ib_send_wr

[Sagi Grimberg]

On 8/4/2015 10:16 AM, Christoph Hellwig wrote:
> This patch split up struct ib_send_wr so that all non-trivial verbs
> use their own structure which embedds struct ib_send_wr.  This dramaticly
> shrinks the size of a WR for most common operations.

Hey Christoph,

I think this looks good in general.

Question though, a ULP may want to keep a couple of WRs around instead
of having each allocated in the stack and handled one by one. We need
to provide it with a hint of what is the size it needs.

I just posted a patch to do that in iser 
(http://www.spinics.net/lists/linux-rdma/msg27632.html).

So if I would want to preallocate an array of work requests, what is the
size of the space I'd need?
is it some form of max(sizeof(struct ib_send_wr),
                        sizeof(struct ib_fastreg_wr),
                        sizeof(struct sig_handover), ..)?

>
> This is just a WIP with basic testing for now.  While all in-tree drivers
> except for iSER and iSERt are converted testing coverage is very limited.
>
> For iSER I could really use a helping hand as the WR usage is rather
> confusing..

I can do it. no problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] rdma: split struct ib_send_wr

[Christoph Hellwig]

On Tue, Aug 04, 2015 at 08:06:16PM +0300, Sagi Grimberg wrote:
> Question though, a ULP may want to keep a couple of WRs around instead
> of having each allocated in the stack and handled one by one. We need
> to provide it with a hint of what is the size it needs.

Note that with the drastic shrink of the struct size the typical case
of needing a fast_reg + one send or a few rdma wr should be just fine
on stack now.  Even more so with your registration cleanups which
will drastically shrink the size of the fast reg mrs again.

> I just posted a patch to do that in iser 
> (http://www.spinics.net/lists/linux-rdma/msg27632.html).

That's actually part of the reason why I didn't manage to convert iser
as it seems so convoluted..

> So if I would want to preallocate an array of work requests, what is the
> size of the space I'd need?
> is it some form of max(sizeof(struct ib_send_wr),
>                        sizeof(struct ib_fastreg_wr),
>                        sizeof(struct sig_handover), ..)?

Preferably you'd preallocate them in a type safe manner, if you really
need to overlay them do it as a proper union.  For example the rds
code is doing that although it only preallocates on per work item.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] rdma: split struct ib_send_wr

[Sagi Grimberg]

On 8/4/2015 8:23 PM, Christoph Hellwig wrote:
> On Tue, Aug 04, 2015 at 08:06:16PM +0300, Sagi Grimberg wrote:
>> Question though, a ULP may want to keep a couple of WRs around instead
>> of having each allocated in the stack and handled one by one. We need
>> to provide it with a hint of what is the size it needs.
>
> Note that with the drastic shrink of the struct size the typical case
> of needing a fast_reg + one send or a few rdma wr should be just fine
> on stack now.  Even more so with your registration cleanups which
> will drastically shrink the size of the fast reg mrs again.

I do agree that the size on the stack is less of an issue now. What
still can matter is handling each wr one by one vs. doing a collective
post.

>
>> I just posted a patch to do that in iser
>> (http://www.spinics.net/lists/linux-rdma/msg27632.html).
>
> That's actually part of the reason why I didn't manage to convert iser
> as it seems so convoluted..

I can understand that. In the last patches I tried to simplify the flow
(I noticed that these aren't in your tree correct?). There always room
for cleanups... I'm trying to have a gradual progress towards getting it
to be simpler.

>
>> So if I would want to preallocate an array of work requests, what is the
>> size of the space I'd need?
>> is it some form of max(sizeof(struct ib_send_wr),
>>                         sizeof(struct ib_fastreg_wr),
>>                         sizeof(struct sig_handover), ..)?
>
> Preferably you'd preallocate them in a type safe manner, if you really
> need to overlay them do it as a proper union.

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

Re: [PATCH, RFC] rdma: split struct ib_send_wr

[Christoph Hellwig]

On Tue, Aug 04, 2015 at 08:44:26PM +0300, Sagi Grimberg wrote:
> I do agree that the size on the stack is less of an issue now. What
> still can matter is handling each wr one by one vs. doing a collective
> post.

But if structured correctly you can still do that with on-stack WRs.

> I can understand that. In the last patches I tried to simplify the flow
> (I noticed that these aren't in your tree correct?). There always room
> for cleanups... I'm trying to have a gradual progress towards getting it
> to be simpler.

I used Doug's to be rebased for 4.3 tree from yesterday or so.  So
whatever he merged is in.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH, RFC] rdma: split struct ib_send_wr

[Sagi Grimberg]

On 8/4/2015 8:47 PM, Christoph Hellwig wrote:
> On Tue, Aug 04, 2015 at 08:44:26PM +0300, Sagi Grimberg wrote:
>> I do agree that the size on the stack is less of an issue now. What
>> still can matter is handling each wr one by one vs. doing a collective
>> post.
>
> But if structured correctly you can still do that with on-stack WRs.

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