[Stgt-devel] [PATCH 03/20] iser transport buf
Wed Nov 14 08:33:32 CET 2007
On Tue, 13 Nov 2007 19:36:05 -0500
Pete Wyckoff <pw at osc.edu> wrote:
> tomof at acm.org wrote on Mon, 12 Nov 2007 23:14 +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.
> > I killed memory allocation in scsi device code (spc, mmc, sbc, smc)
> > and kill all the hacky memory management code. Now LLDs allocate and
> > free buffer for data transfer.
> But now you need two mallocs per command: one for the task and one
> for the (aligned) data. Bidi needs three. This will show up in
> iser performance, but might be a worthwhile trade-off. Maybe we
> could add slabs for common sizes (ick).
I thought that iSER code already does something like that since you
use pre-registered buffer again and again.
glib also does something like that. So I don't think that this change
will effect performance much.
> I have a lot of rebasing to do after all your bidi buffer work. It
> looks like good changes so far, but I have a couple of concerns from
> a brief peruse of the changes in git.
> Zeroing out the data chunk for non-READ/WRITE commands. There are
> lots of OSD commands hidden inside the 0x7f extended format, and
> other non-OSD non-SBC commands too. Can we just move the zeroing to
> where it is needed? Like in each of the inquiry, etc. This zeroing
> was a major performance problem for iser.
Yeah, I added that hack because device type code is broken (assumes
that buffer is large enough and zero'ed out). We need to fix device
> Accessor and setter functions for scmd. I don't see what value they
> add. Except maybe for the "(void*)(unsigned long)" ones.
I want to insulate LLDs from scsi_cmd internals like Linux scsi-ml
does. It enables us to change the structure freely in the future
without caring about device type code and llds.
> Also, building functions using cpp to glue names together breaks
> things like ctags, cscope, and diff -p. Curious to hear why you
> like them.
Yeah, there are the pros and cons of using cpp magic. I prefer using
cpp magic than defining lots of similar functions. And I think that
linux scsi developers are already familiar with that (see the
> Thanks for working on this. Bidi handling in bs_* will be much
> cleaner now.
No problem. I wanted this cleanups before the code becomes bigger by
adding iSER, FCoE, etc.
> I'll be away from email until 25 nov, but will look at
> your changes in detail and rebase iser after.
Thanks. Until 25, I try to merge some of your iSER patches that
haven't been merged yet.
More information about the stgt