Re: [RFC 1/2] scsi core: alloc_cmnd

[Matthew Wilcox <matthew@wil.cx>]

On Tue, Oct 23, 2007 at 06:49:17PM +0200, Boaz Harrosh wrote:
> You know Matthew when you first talked about this, I envisioned
> something else.

Right, so did I.  Christoph felt that alloc_cmnd/destroy_cmnd a la the
VFS layer's alloc_inode/destroy_inode was a better route.  I didn't have
a strong feeling, so I implemented that.

> This serves the drivers well because all they need to do is set
> host_cmnd_extra_bytes = size_of(my_cmnd_stuff) and start using
> it. much more simple than setting up cache pools. It also keeps
> the Q per host vs Q per driver.

The allocation pool is currently for the whole subsystem, not per host.
I assumed you meant 'pool' rather than 'queue', since I didn't touch the
queues.

Hosts already have allocation pools (er, I think they all do anyway
... and any that don't, I'm quite happy to write a generic_alloc_cmnd
and stick it in scsi_lib).

> This also solves your problem with locks and allocation flags
> since nothing changes, and it is all private to the mid-layer.

It doesn't solve that problem -- we already have that problem; it's just
that it's kept in the midlayer.

> And it serves me, because when bidi comes I just do:
> sizeof(struct scsi_cmnd) + host_cmnd_extra_bytes + sizeof(bidi_stuff)
> for hosts that support bidi. And the rest of the work was done by you.

I have to admit to not having a clear picture of how bidi is supposed to
work.  Could we do it something like this?

struct bidi_cmnd {
	struct scsi_cmnd;
	...
};

struct qla_cmnd {
	struct bidi_cmnd bd_cmnd;
	...
};

We could also add an alloc_bidi_cmnd/destroy_bidi_cmnd to the shost
template.  Presumably most commands won't be bidi for any given host,
so it'd be a waste of space to allocate them for all commands.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."