[openib-general][kdapl]: vmalloc instead of kmalloc

[Guy German]

Hi,

James Lentini <mailto:jlentini at netapp.com> wrote:
> On Tue, 2 Aug 2005, Guy German wrote:
> 
>> Hi Muli,
>> 
>> Muli Ben-Yehuda <mailto:mulix at mulix.org> wrote:
>>> On Tue, Aug 02, 2005 at 06:24:49PM +0300, Guy German wrote:
>>> 
>>>> There are some places where kmalloc might not be enough :
>>>> in dapl_evd_event_alloc there is an allocation:
>>>> 
>>>> 	event = kmalloc(evd->qlen * sizeof *event);
>>>> 
>>>> whereas evd->qlen can be 128k (depends on max_cqe of the hca) and
>>>> kmalloc would fail. 
>>>> 
>>>> The same goes to dapl_rbuf_alloc.
>>>> 
>>>> Is it legit to replace those kmallocs with vmallocs ?
> 
> We should only add calls to vmalloc() as a last resort. As
> Muli points
> out, they are discouraged.
> 
>>> Why do we need such a large allocation?
> 
> kDAPL creates two large pools of memory.
> 
> One is for events. When the kDAPL consumer creates an EVD, it
> specifies a queue size (the number of events the EVD can hold). The
> implementation pre-allocates a pool of events equal to the
> size of the
> queue. These events are used when an IB upcall is made (e.g.
> connection request, connection established, aysnc. error,
> etc.) or the
> kDAPL consumer posts a "software event" via dat_evd_post_se().

And, of course  - completions of data events - which is why the queue
need to be more substantial.

> The other memory pool is for cookies. A kDAPL event contains certain
> fields that the IB work completion (ib_wc) does not provide (like the
> EVD, EP, etc.). For that reason, the kDAPL provider sticks
> the missing
> information in a dapl_cookie structure and sets it as the work
> request's context value. When the work completion comes back, the
> kDAPL provider pulls the cookie out and uses it to populate the
> missing event fields. These cookies are also pre-allocated in a pool
> equal to the EVD size. 
> 
>>> To answer your question, vmalloc has a performance overhead and can
>>> and will fail when vmalloc-space is exhausted (as can kmalloc, for
>>> different reasons). Can this allocation be cut down so that it
>>> becomes a non-issue?
> 
> The size of the event pool seems much larger than necessary. I would
> expect most consumers only use a few events from this pool (with no
> errors or software events, a client will use 2 and a server will use
> 3). 

If you consider that the consumers are polling from the queue themselves
(upcall policy is disabled, for performance) and the queue of events holds
completions of data, then you have to support larger queue.
Bare in mind that one target can have many initiators.

Any way, ISER seems to be needing a solution for this, and I think it is
possible to come up with a different solution than vmalloc (maybe a few
kmallocs) I will think about it and send a patch when I have one.
 
> We may be able to eliminate the cookie pool entirely. There
> are only a
> few values we need from the cookie. I'll look into that.

That sounds good.

BTW, I've calculated the sizeof evd rather then dat_event, which is actually 48.
That still leaves ~2730 possible pending events, which is basically the same principle.

Guy.

>> evd_min_qlen defines the size of the event queue that the Consumer
>> requested. sizeof *event = 184 - that leaves ~712 pending events,
>> which is not much. ISER target is trying to support about 5000 (by
>> their 
> calculations), but other consumers
>> might want to support even more and there is no reason for
> dapl to limit what the ib can provide.
>> Note that iser dequeues the events itself (only the first
> event is accepted from a callback), hence the
>> need for a normal size queue.