On Wed, Oct 19, 2016 at 12:45:31PM +0100, Andrew Cooper wrote:
> On 19/10/16 11:46, Wei Liu wrote:
> > If it is present, use it; otherwise use 4096. Also provide two new
> > macros to let user have control over the page size used to do
> > calculation.
> >
> > Signed-off-by: Wei Liu <wei.l...@citrix.com>
> > ---
> > Cc: Andrew Cooper <andrew.coop...@citrix.com>
> > Cc: George Dunlap <george.dun...@eu.citrix.com>
> > Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> > Cc: Jan Beulich <jbeul...@suse.com>
> > Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> > Cc: Stefano Stabellini <sstabell...@kernel.org>
> > Cc: Tim Deegan <t...@xen.org>
> > Cc: Wei Liu <wei.l...@citrix.com>
> > Cc: Juergen Gross <jgr...@suse.com>
> > ---
> > xen/include/public/io/usbif.h | 24 ++++++++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/include/public/io/usbif.h b/xen/include/public/io/usbif.h
> > index 4053c24..ac38318 100644
> > --- a/xen/include/public/io/usbif.h
> > +++ b/xen/include/public/io/usbif.h
> > @@ -216,6 +216,16 @@ struct usbif_urb_request {
> > };
> > typedef struct usbif_urb_request usbif_urb_request_t;
> >
> > +/*
> > + * Reference to PAGE_SIZE was overlooked when this header file was
> > + * introduced. Respect PAGE_SIZE if defined, otherwise, use 4096.
> > + */
> > +#ifdef PAGE_SIZE
> > +#define _USB_RING_PAGE_SIZE PAGE_SIZE
> > +#else
> > +#define _USB_RING_PAGE_SIZE 4096
> > +#endif
>
> Tokens starting with an single underscore and a capital letter are reserved.
>
Right, this can be fixed.
> Also, I am -1 to screwing around like this. The header file should not
> reference PAGE_SIZE as it creates ABI-incompatible code when compiled on
> different systems.
I also dislike keeping PAGE_SIZE here, but what if existing code really
expects such behaviour? :-/
I don't know enough about the history and / or users of these macros
TBH. I trust Juergen's judgement because he knows more about this than
me -- both this and vscsi came from SuSE and predates upstream PVOPS
kernel AFAICT.
>
> If we really want to get into a rant, then we *should not* be using C to
> describe our ABI in the first place, for precisely reasons like this.
> ABIs should be written down in a text document. It is fine to provide C
> header files to implement an ABI described elsewhere, but using C as the
> ABI statement causes repeated disasters like this.
>
Agreed.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
