[Stgt-devel] [PATCH 04/20] iser bidi alloc read buf

[Pete Wyckoff]

tomof at acm.org wrote on Sat, 27 Oct 2007 23:56 +0900:
> On Tue, 16 Oct 2007 11:19:03 -0400
> Pete Wyckoff <pw at osc.edu> wrote:
> 
> > Allocate the read buffer for bidirectional commands in the transport to
> > pass down to devices.  A device can fill and return this buffer in
> > task->uaddr, and thus choose to do the read or write processing in any
> 
> Where do you free bidi buffer?

The bs sets cmd->uaddr to be the data-in buffer (from initiator
point of view) as it returns.  iscsi puts this in task->addr then
later frees it if (task->addr != task->data).  Yeah, kind of
complex.

Write command:  single allocation: task plus data-out all in one go.
    bs is given task->data, does not change cmd->uaddr.  Single free
    of task releases it.

Read command:  single alloction:  task plus expected data-in size.
    bs is given task->data, fills it, does not change cmd->uaddr.
    Single free.

(Inquiry etc look like read, and used to return a new cmd->uaddr,
    requiring a second free.  But now use given task->data after
    iser-transport-buf is applied.)

Bidi command:  two allocations: task plus data-out, expected
    data-in.  bs is given task->data as cmd->uaddr, and the
    buffer for the data-in as cmd->bidi_uaddr.  It sets cmd->uaddr
    to cmd->bidi_uaddr as it finishes, then iscsi frees that and
    the task.

Before this patch, bidi started out like write, but then the bs
would malloc a new buffer for the read result and return that in
cmd->uaddr.  iscsi_free_task() sees task->addr != task->data and
frees that as well as task, just like old inquiry implementation.
Change here is to make sure we can use a preallocated buffer from
the transport if one is given.

> Please don't forget that bidi support isn't only for iSER. everyone
> needs it.

Yes, we got bidi working back in March 2007 and use it every day,
without iSER.  Unfortunately the only bidi user is in our
out-of-tree OSD code.  We're releasing the software OSD target
library soon, then could add the backing store code to stgt,
dependent on the presence of that library.

> The current buffer management is not clean so feel free to rewrite it
> before iSER if you want.

I would be willing to do this, but don't have a better suggestion
yet.  Any ideas welcome.  Difficulty is supporting both bidi-aware
and non-aware bs, and pre-malloc and non-pre-malloc transports.  All
while minimizing the number of malloc operations per command.

> > order.  Unfortunately, this allocation can not be combined with the task
> > and task->data allocation as the bidi read size is not known until after
> > AHS processing.
> 
> We need to change task->data allocation since we need aligned buffer
> for DIO. The iSCSI driver uses page cache buf IET experience tells
> me that we need to support DIO too.

As in bs_xen?  Do you think the DIO alignment needs could fit into
the framework described above?  Or does the BS somehow need to tell
the transport what to allocate, to get alignment space?  That would
definitely affect any proposed buffer mgmt rewrite.

		-- Pete