svn commit: r316648 - in head/sys: amd64/amd64 amd64/include arm/arm arm/include arm64/include cddl/dev/dtrace/aarch64 cddl/dev/dtrace/amd64 cddl/dev/dtrace/arm cddl/dev/dtrace/i386 cddl/dev/dtrace...

[John Baldwin]

On Monday, April 10, 2017 04:26:03 PM Patrick Kelsey wrote:
> On Mon, Apr 10, 2017 at 1:43 PM, John Baldwin <jhb at freebsd.org> wrote:
> 
> > On Monday, April 10, 2017 01:23:04 PM Jung-uk Kim wrote:
> > > On 04/08/2017 22:00, Patrick Kelsey wrote:
> > > > Author: pkelsey
> > > > Date: Sun Apr  9 02:00:03 2017
> > > > New Revision: 316648
> > > > URL: https://svnweb.freebsd.org/changeset/base/316648
> > > >
> > > > Log:
> > > >   Corrected misspelled versions of rendezvous.
> > > >
> > > >   The MFC will include a compat definition of
> > smp_no_rendevous_barrier()
> > > >   that calls smp_no_rendezvous_barrier().
> > > >
> > > >   Reviewed by:      gnn, kib
> > > >   MFC after:        1 week
> > > >   Differential Revision:    https://reviews.freebsd.org/D10313
> > > ...
> > >
> > > We knew about the problem but we didn't fix it because it breaks KPI.
> > > For example, sysutils/virtualbox-ose-kmod.  If you really want to MFC
> > > this change, you have to implement shims.
> >
> > Also, the function isn't actually called, but is only used in comparisons
> > in smp_rendezvous_action().  To do a compat shim you will need to either
> > change these comparisons to compare against both function pointers or
> > define the alternate symbol as an alias of the existing function.  That
> > only helps the KBI though.  For the KPI would just use a #define to point
> > to the new name.
> >
> 
> That's a good point about the comparisons in smp_rendezvous_action() - if I
> had managed to miss that detail all the way through the compat shim
> implementation, it would have littered pointless empty function invocations
> and atomic increments into all the uses of smp_rendezvous() that used
> smp_no_rendezvous_barrier for at least one of the stages.
> 
> I don't think we have an established place to define machine-independent
> symbol aliases.  Approaching that through linker scripts would require
> spamming a PROVIDE() statement into each of the arch-specific scripts.
> Since this is a function, I think a better way than the symbol alias +
> #define approach would be to just define a function pointer called
> smp_no_rendevous_barrier that gets statically initialized to
> smp_no_rendezvous_barrier.  In that case, the extern decl takes care of the
> KPI, the corresponding symbol definition takes care of the KBI, and it has
> a minimal, MI, code footprint.

I was thinking something along the lines of __weak_reference() from
sys/cdefs.h.   Actually, maybe
__strong_reference(smp_no_rendezvous, smp_no_rendevous_barrier)
would be sufficient?

> Either that, or I relax the goal of purging it entirely and put the compat
> shim in current also to sidestep the port-patching issue.

Well, I'd like to not have it live around forever, but we could add the
compat shim to HEAD in the short term (also makes MFC slightly simpler as
you just group the two commits together when you MFC).  Once the port is
patched with the updated FreeBSD_version we can remove the shim from HEAD.

-- 
John Baldwin