Re: [Xen-devel] [PATCH 13/25] argo: implement the register op

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.12.18 at 06:41, <christopher.w.clark@xxxxxxxxx> wrote:
> On Wed, Dec 12, 2018 at 8:48 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>>
>> On Fri, Nov 30, 2018 at 05:32:52PM -0800, Christopher Clark wrote:
>> > +static inline uint16_t
>> > +argo_hash_fn(const struct argo_ring_id *id)
>>
>> No need for the argo_ prefix for static functions, this is already an
>> argo specific file.
> 
> Although the compiler could live without the prefix, I'm finding it helpful to
> very easily determine that functions being used are not defined elsewhere
> within Xen; so I've left the prefix as is for version two of this series.

But you realize that this needlessly increases the string table
size as well as the volume of output going over the (normally
low bandwidth) serial line in case of an isuse?

>> > +/*
>> > + * Messages on the ring are padded to 128 bits
>> > + * Len here refers to the exact length of the data not including the
>> > + * 128 bit header. The message uses
>> > + * ((len + 0xf) & ~0xf) + sizeof(argo_ring_message_header) bytes.
>> > + * Using typeof(a) make clear that this does not truncate any high-order 
>> > bits.
>> > + */
>> > +#define ARGO_ROUNDUP(a) (((a) + 0xf) & ~(typeof(a))0xf)
>>
>> Why not just use ROUNDUP?
>>
>> And in any case this shouldn't be on the public header IMO, since it's
>> not part of the interface AFAICT.
> 
> Well, in version two it's now: XEN_ARGO_ROUNDUP :-)
> because it does need to be in the public header because it's used within the
> Linux device driver, and items in that public Xen header need the 'xen' prefix
> (so they now do).  Within the Linux code, it's used to choose a sensible ring
> size, and also used when manipulating the rx_ptr on the guest side.

It's at least questionable to put constructs into the public header
that the header itself doesn't use, and the naming of which may
not be what consumers would prefer. I can see the "single
central place" point though.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel