libthr shared locks

[Konstantin Belousov]

On Mon, Dec 28, 2015 at 11:59:02AM -0500, Daniel Eischen wrote:
> On Mon, 28 Dec 2015, Konstantin Belousov wrote:
> 
> > On Sun, Dec 27, 2015 at 11:44:44AM -0500, Daniel Eischen wrote:
> > Adding the self-pointer is good idea, but it would not work for the
> > shared locks.  If you are going to work on this, then I will scratch
> > my patch, to not impede on your work.
> 
> Shared locks would work, but only for the new ABI (I guess that
> would be FBSD1.4 in HEAD, or FBSD1.5 if we bumped it again just
> for this).
I only mean that self-pointer sometimes would cause additional issues.

> 
> Another option is to just increase the size for the pthread synch
> types without really changing the first element (still a pointer
> to the allocated object - the implementation would stay the same).
> The only breakage would then be storage layout issues for new
> compiles linking against older ones.  For FreeBSD-12, we could
> change the implementation to a real inlining (without changing
> the storage size), that gives a major release cycle for 3rd
> parties to make the change.  I'm not sure if this would really
> be much of a benefit - the storage size change alone would
> probably be the greatest pain.
It is a good intermediate point in the whole plan of changing the ABI.
The work required to support pshared after the size is increased is not
completely trivial, but is not a rocket science either. But your note
about intermediate step might give additional freedom in executing the
whole plan.

> 
> > Taking out the inlining bits from the David patch, or (which would I do,
> > if doing this) just reimplementing it from scratch is easy enough and
> > just require some time.  I estimated this job to take between one and
> > two weeks.
> 
> I think a lot of David's patch is the renaming of all the
> elements of the public structs to prepend '__'.  I was thinking
> it would be nice to have the public structs be something like
> this:
> 
>    struct pthread_mutex_t {
>        uint32_t __x[IMPL_REQ + IMPL_SPARE + pad_to_CACHE_LINE_SIZE];
>    };
> 
> and then have libthr override the definition.  That would
> make declaring PTHREAD_MUTEX_INITIALIZER, etc, a little
> magical, but avoid a lot of needless churn in libthr.
This is very good suggestion, I fully agree.  There are some more details,
e.g. it would be better to use uint64_t or explicit align attribute, to
get proper alignment, but overall idea is sound, of course.

> 
> But, I agree, just doing the inlining is basic grunt work
> and limited in duration.  Your estimate seems reasonable.
> 
> > What is really thrilling is to manage the consequences of the ABI
> > breakage. When I did the evaluation for the FF project, I put a six
> > months extent for the whole work. This is for things like looking at
> > the ports impact, trying to know in advance where mixed things start to
> > break, monitoring the users complains about issues caused by the ABI
> > break etc.
> 
> There's not much we can really do to manage it, just make it
> known that all ports need to be recompiled on 11 in order to
> be on the safe side.  The libmap.conf trick will not work
> either.
No.  This is probably the point where our positions diverge most.

I do think that some mitigation measures can be applied, e.g. your
self-pointer trick, check for presence of 'other' lib when loading
given threading library, renaming etc.  It should be thought out in
advance.

Another thing to do is to monitor mailing lists, react to the user
issues, see if it is related to the ABI breakage, and possibly create
another measures which would make some issues less painful.

I consider both activities absolutely vital when doing this surgery.
In principle, I may do both, but it requires more planning and is
not immediately reachable comparing with the off-page approach to
only get pshared right now.

> > You see my position, I would avoid ABI breakage at all costs. If I
> > cannot talk you against this, please do not consider the work done after
> > the inlining patch is committed to HEAD, it is actually only start
> > there.
> 
> Ok, I do think we want to do it at some point.  Perhaps 12-current
> would be better...

Some people in private also told me that they consider 11 too risky target
for this change.  We will see.