[Stgt-devel] [PATCH 03/20] iser transport buf

[Pete Wyckoff]

tomof at acm.org wrote on Sat, 27 Oct 2007 23:56 +0900:
> On Tue, 16 Oct 2007 11:18:57 -0400
> Pete Wyckoff <pw at osc.edu> wrote:
> 
> > For RDMA, it is often nice to use data from a pool of pre-registered
> > buffers.  To do this, the transport allocates memory for a response and
> > passes it down to the devices to fill.  Some operations, though,
> > allocate their own buffers and return that new memory instead.  These
> > are usually small and the allocation is just done for convenience to
> > avoid length bounds checking.  Copy the data into the provided transport
> > buffer instead.
> 
> Do you really need pre-registered buffers for INQUERY, non I/O
> commands?

To send the data, it must be in a registered buffer.  We preregister
some for this purpose.  The other way is to dynamically register
the buffer, then deregister it after tha data has been transferred.
This adds lots of overhead, especially in the small IO case (like
inquiry), and code complexity.

Currently iscsi_alloc_task() uses malloc() to get a buffer big
enough for the incoming command and any data that will be returned
such as the inquiry result.  The approach in RDMA is to make sure
that the allocation comes from the preregistered area.  TCP just
uses malloc() as before.

IO operations like in bs_sync will copy data into the cmd->uaddr
(same as iscsi's task->data).  Some inquiry-like routines allocate a
page-size buffer themselves with valloc() to avoid lots of little
bounds checks, and return this instead of using the provided
cmd->uaddr.  But this avoids our preregistered buffer space.

So the new approach in these inquiry-like routines is to continue to
valloc() a page to hold the generated data and fill it, but then
invoke a new helper "spc_return_buf()" that copies data from the
valloc()-ed area into cmd->uaddr.

> Using pre-registerd buffers might make the code simpler than handling
> both pre-registerd and normal buffers, but we already need to handle
> something like that for mmapped I/Os.

Only bs_mmap handles cmd->mmaped.  It appears to be not for use with
iscsi.

The modifications to inquiry only happen if cmd->uaddr is non-NULL.

> > Also fixes some leaks of these extra buffers in error paths, and cleans
> > up unreachable code in ibmvio inquiry.
> 
> Can you send a separate patch to do that?

Split up into two prior patches.

		-- Pete