Re: [Xen-devel] [PATCH] include/public/io/ring.h: Remove xen_mb, xen_rmb, xen_wmb macros

[Juergen Gross <jgross@xxxxxxxx>]

On 05.07.19 09:59, Jan Beulich wrote:
> On 04.07.2019 18:11, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Jan Beulich <JBeulich@xxxxxxxx>
> > > Sent: 04 July 2019 16:49
> > > To: Anthony Perard <anthony.perard@xxxxxxxxxx>
> > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; 
> > > Konrad Rzeszutek Wilk
> > > <konrad.wilk@xxxxxxxxxx>; Juergen Gross <JGross@xxxxxxxx>
> > > Subject: Re: [Xen-devel] [PATCH] include/public/io/ring.h: Remove xen_mb, 
> > > xen_rmb, xen_wmb macros
> > >
> > > On 04.07.2019 17:15, Anthony PERARD wrote:
> > > > Those macros where introduced when a prefix "xen_" was added to
> > > > mb,rmb,wmb. There are gated on __XEN_INTERFACE_VERSION__, but there
> > > > are not part of the Xen interface. Users of ring.h needs to provide
> > > > xen_[rw]?mb() anywai because [rw]?mb() isn't likely to exist.
> > > It's not clear to me what you want to achieve:
> > The issue is that any project importing this header (in this case QEMU,
> > but I have the same issue in the Windows PV drivers) needs to import
> > xen-compat.h (or dream up a header of the same name), even though this
> > header is only concerned with the underpinnings of PV protocols and has
> > nothing, as such, to do with Xen.
> While I agree this shouldn't have been part of the public headers,
> that ship has sailed long, long ago. If a component doesn't use the
> headers verbatim, I don't see why they couldn't remove that section
> in their copy. If otoh they use the headers verbatim, then I'd
> expect them to also use xen-compat.h
Right.

> > To keep old verbatim users (are there really any at all?) happy, how about 
> > simple...
> >
> > #ifndef xen_mb()
> > #define xen_mb() mb()
> > #endif
> >
> > constructs?
> This would still cause conflicts if a component ends up defining
> xen_mb() only after the inclusion of this header. As to there
> actually being any - the old Linux 2.6.18 tree did pull in copies
> of the headers without further editing. Beyond that while I'm
> unaware of any, we simply can't know. Until now there simply was
> an implied promise that we would try our best to avoid breaking
> existing users.
I'm completely on Jan's side here.

What would be possible perhaps is to split ring.h into two headers: a
new one with the pure ring definitions and ring.h #include-ing
xen-compat.h and the new header and #define-ing the xen*mb() macros.

Not sure this would be worth it, though.


Juergen

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