Re: [Xen-devel] [PATCH 1/4] Add XEN pvSCSI protocol description

[Juergen Gross <jgross@suse.com>]

On 06/30/2014 01:02 PM, Jan Beulich wrote:
>>>> On 27.06.14 at 19:11, <konrad.wilk@oracle.com> wrote:
>> On Fri, Jun 27, 2014 at 04:34:33PM +0200, jgross@suse.com wrote:
>>> +/*
>>> + * Maximum scatter/gather segments per request.
>>> + *
>>> + * Considering balance between allocating at least 16 "vscsiif_request"
>>> + * structures on one page (4096 bytes) and the number of scatter/gather
>>> + * elements needed, we decided to use 26 as a magic number.
>>> + */
>>> +#define VSCSIIF_SG_TABLESIZE		26
>>> +
>>> +/*
>>> + * based on Linux kernel 2.6.18
>>
>> This being a bit more .. new, - do these sizes make sense anymore?
>> Should they be extended a bit? Or have support for using the
>> old ones (as default) and then negotiate new sizes with the kernel?
>> (If of course there is a difference?)
>
> As Jürgen already said (and as you should have noticed yourself) -
> this is an interface definition that we can't just change. Negotiation
> of larger counts is an option, which is what VSCSIIF_ACT_SCSI_SG_PRESET
> is intended for (the implementation of which isn't part of this patchset
> afaics, but could be made available on top of it).

I don't think this is the proper way to handle larger SG lists. The
VSCSIIF_ACT_SCSI_SG_PRESET option would just bump the maximum length
up to 31 (currently 26). Or was it thought to be incremental (multiple
presets for one request)? I'd rather add a way to specify SG lists
residing in an own (granted) page (or even multiple pages). This would
allow 512*26 SG entries without having to change the request structure.
The capability to handle this feature could be indicated via xenstore.

> Most if not all of your other comments are a little questionable too
> in this context - any parts you aren't happy about would really
> better be addressed towards the canonical header in the Xen tree.
> (I know you had other interface headers diverge too in their Linux
> incarnation, but personally I don't think this is an appropriate thing
> to do, perhaps apart for pure coding style adjustments.)


Juergen

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html