libthr shared locks

[Daniel Eischen]

On Mon, 28 Dec 2015, Konstantin Belousov wrote:

> On Sun, Dec 27, 2015 at 11:44:44AM -0500, Daniel Eischen wrote:
>> On Sun, 27 Dec 2015, Konstantin Belousov wrote:
>>
>>> On Sat, Dec 26, 2015 at 12:15:43PM -0500, Daniel Eischen wrote:
>> [ snipped for brevity ]
>>>>
>>>> I agree, but the work that you are doing now would be basically
>>>> thrown out later on.  I will not stand in your way and appreciate
>>>> any work you do.  I would just rather that the struct change be
>>>> made now for 11, even without any pshared or other changes.  For
>>>> once the struct change is made, pshared or other additions can
>>>> be made afterward, even in the 11 branch because they would not
>>>> break the ABI.
>>>
>>> Lock inlining was not done for ten years, now cost of doing it is
>>> extremely high, as discussed above.  Who would drive the change, and
>>> with what time frame ?  If me, I seriosly consider renaming libthr
>>> to libthr2, but I had no time to think much about it.
>>
>> I could probably do the inlining of locks, pulling out that part
>> of it from David's patch, and coordinating with you so we don't
>> step on each other's toes.  Perhaps just making the first element
>> of the structs a self-reference at first, would help mitigate
>> that...
> 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).

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.

> 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.

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.

If we could instrument libthr or rtld to emit a warning/error
message with a URL to a FAQ or the the src/UPDATING entry that
would be nice.

> We would see afterward if I overestimated the work, but I do the typical
> mistake of underestimating usually, whatever insane large the initial
> numbers are.
>
>>
>> I'm not sure what renaming libthr to libthr2 solves that a
>> version bump can't also.  Can't we still tell whether both
>> libthr.so.3 and libthr.so.4 have been loaded?  Perhaps libthr2
>> is cleaner WRT keeping the old ABI (it could be dropped?).
> Yes, part of the change probably should be a prevention of simultaneous
> existence of libthr.so.3 and (libthr.so.4 or libthr2.so.1, whatever
> it is named) in one process. This should be done either with rtld
> facilities, I am not sure how. Or e.g. with dlopen("otherlib",
> RTLD_NOLOAD) in the constructor of each library and failing if dlopen()
> returns a valid handle.
>
> Another immediate point, not only libthr.so must be bumped, but also all
> base libraries depending on it must be.  Even if some libraries do not
> directly record the dependency, they might require the bump as well, I
> am thinking about c++ runtime.  This should allow the compat packages
> to provide useable libraries.
>
> IMO this is easier to see when the libraries names differ significantly
> in the name part, and not in the .so.n part.  At least users would be
> less surprised, but also the work of the person who tracks the deps,
> would be easier too.

>>> Right now, I think that I want to commit my current patch and implement
>>> robust mutexes as the next step, without ABI breakage. At least, this
>>> seems to have fixed time-frame and can be made ready for 11.x. Lock
>>> inlining might be not. Are there serious objections against the plan,
>>> except that (lock inlining + pshared) is ideal situation, while the plan
>>> is not (but more practical) ?
>>
>> What is the timeframe for 11.0-RELEASE?  If not for 11.0, I would
>> like to see it done soon in the 12-current branch afterward.
>> In my mind, any pain will be the same in 11 or 12, nothing really
>> is gained by waiting, only by not doing it (inlining locks) ever.
>
> Difference is in causing more (ABI) breakage on the near-coming
> stable/11 users, or trying to fix or mitigate more of it in HEAD,
> using the local population as the canaries.
>
> 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...

-- 
DE