Re: [COMMITTED PATCH] Split timed-wait functions out of nptl/lowlevellock.c.

[Roland McGrath <roland at hack dot frob dot com>]

> It seems you made the split just to support the absolute timeout path
> that NaCl futexes provide out of the box.  IMO, this should have been
> addressed by fixing the glibc-internal futex API, not by adding a
> NaCl-specific variant and lots of new dummy files just to get back to
> the common implementation for all other archs.  Didn't we agree to have
> as much commonality in our synchronization code as possible?
> 
> I'd appreciate if we could have discussion on such patches in the future
> before they are committed.  (And I wouldn't mind at all if you'd revert
> this patch.)

In fact, the NaCl-specific __lll_timedwait_tid change was the real
motivation and the NaCl-specific __lll_timedlock_wait (which shouldn't
need to be, as you note) was just nearby low-hanging fruit.

The actual change in this thread was purely splitting one file of
multiple functions into more files of fewer functions each.  That is
simply moving things in the direction that they traditionally always
were and always should be in libc: separate functions in separate files.
One of the reasons that's the good way to be is that it makes more,
finer grains that can be overridden in sysdeps/ variants without
duplicating the source code that doesn't need to vary.  But that's not
the only reason to do it.  (Avoiding dead code in static linking is the
other concrete one, though that might well not apply usefully here.)

It's a proper cleanup regardless of whether one needs or wants to
actually implement new sysdeps/ variants of any of the functions in
question.  Such pure cleanup changes never need nontrivial review, and
I won't apologize for committing them without waiting for feedback
(unless I break a build or whatever, which was not the case here).

Once the file was split up, it was very easy to add
sysdeps/nacl/lll_timedlock_wait.c and save some suboptimal stupidity
that had become hard for me to pretend I didn't see.  I have no
general expectation that anybody will review changes I make purely in
sysdeps/nacl/ code, though I'm certainly grateful for any feedback
anybody wants to give on that code.  So there was no reason to wait
before committing sysdeps/nacl/lll_timedlock_wait.c.  Just because
it's not the ideal solution doesn't mean it wasn't worth doing when I
did it.  It certainly will be simple enough to remove it again when it
is no longer adding any benefit, and I fully intend to do that.


Thanks,
Roland