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

[Guy German]

James,

I see what you mean.
The allocation of the event vector is derived from evd->qlen.
In DTO ev'd, however, qlen is also the parameter passed to 
ib_create_cq. 
Since we don't want to limit DAPL consumers to an 
unnecessary small completion queue size, maybe we
could differentiate between DTO supporting evd's and 
CONN evd's, when allocating the events vector.

if evd supports CONN only, leave it :
	event = kmalloc(evd->qlen * sizeof *event)
(Relying on the consumer he knows what he is doing)
if evd is DTO only :
	don't allocate an event buffer, at all
if evd supports both :
	event = kmalloc(DEFAULT_4_CONN * sizeof *event)

if DEFAULT_4_CONN=256, that's a 3 pages allocation.

How does that sound to you ?

Thanks,
Guy

James Lentini <mailto:jlentini at netapp.com> wrote:
> On Wed, 3 Aug 2005, Guy German wrote:
> 
>> James Lentini <mailto:jlentini at netapp.com> wrote:
>>> 
>>> 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.
> 
> In uDAPL yes, but not in kDAPL. None of the callers of
> dapl_evd_get_event(), where events are dequeued from the EVD's
> free_event_queue, use the events for DTO completions.
> 
> In uDAPL, dat_evd_wait can dequeue data events and store them in the
> pending event queue. 
> 
> As Itamar pointed out, kDAPL could use a single circular list instead
> of maintaining the EVD's free and pending event queues.
> 
>>> 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.
> 
> Even if the event queue never stores DTO events? In the worst case, I
> agree that kDAPL would need to allocate an amount of memory
> equal to n
> * sizeof(DAT_EVENT), where n is the EVD queue size.
> 
> My observation is that an EVD almost always uses the event pool < 5
> times (when there are no async errors and no software
> events). Further
> more, it usually only uses one event at a time (it posts a connection
> request event, the consumer reaps it, it posts a connection
> event, the
> consumer reaps it, ...). Given that, allocating a event pool equal to
> the queue length seems like overkill to me. The EVD could allocate
> smaller blocks of events in some configurable size. Most of
> the time a
> single poll (say of 25 events) would be sufficient. In the rare case
> when this pool was exhausted, a second one could be
> allocated. If that
> one was used up, a third could be allocated...
> 
>> 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. 
> 
> Ok. That would be great.