Re: pcp updates: libpcp locking and concurrency control changes

"Frank Ch. Eigler" <[email protected]> Sun, 15 Jan 2017 19:49:51 -0500
Newsgroups gmane.comp.sysutils.pcp
Message-ID <[email protected]>
Hi, Ken -

> Note this is a new branch (threadsafe) in my git tree, not the
> master branch.  These changes need review eyes before being pulled
> into the master branch.  [...]

Nice.  Little nits, if I may?

exit() probably doesn't need any /* THREADSAFE */ marking; it's pretty
safe all right.  (We shouldn't call it much or at all from within the
library, really.)

hoststrerror() ... it'd sure be nice to elimiate the boilerplate
PM_LOCK* wrapping of this one at the call site and hide it inside.
But I guess short of thread-local storage, or ({ })
statement-expressions, can't.

opendir() etc. in our usage are probably safe, because we use it only
on transient thread-local opendir dirp's.  No need for extcall locking
there.


>     [...] Additionally the lock ordering of the new local mutexes
>     and __pmLock_extcall ensures these ones are deadlock free.

Yeah, this is simple to show because "__pmLock_extcall" is a "leaf"
lock, beneath which nothing else (in libpcp) is nested.  In
pmns.c/tz.c, shouldn't there be locks around the getenv()'s?  Hm wait,
_pushTZ's says "extcall" already acquired upon entry ... is that
necessary?  Maybe we need an tz-stack specific lock.

deadlock-freeness (for this part) would be true even if you were to
use different locks for the different "extcalls"; by using only a
single one, you're imposing unnecessary serialization.  (e.g.,
getenv() doesn't need one at all, except perhaps if you're racing
against putenv(), and then an env-specific lock would be enough.  It
certainly does not race with grent() etc.)

Anyway, onto the big pay-off libpcp_lock stuff, yey.  You might
consider pulling in pcpfans.git commit e99cd198f since it shows how to
use valgrind/helgrind to do some basic run-time lock-correctness
checking.  Other qa test cases could benefit from it.


- FChE

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links:

You receive all messages sent to this group.

View/Reply Online (#14977): https://groups.io/g/pcp/message/14977
View All Messages In Topic (2): https://groups.io/g/pcp/topic/4185439
Mute This Topic: https://groups.io/mt/4185439?uid=174580
New Topic: https://groups.io/g/pcp/post
-=-=-
pcp mailing list
[email protected]
https://groups.io/g/pcp/messages
-=-=-
Change Your Subscription: https://groups.io/g/pcp/editsub?uid=174580
Group Home: https://groups.io/g/pcp
Contact Group Owner: [email protected]
Terms of Service: https://groups.io/static/tos
Unsubscribe: https://groups.io/g/pcp/leave/354243/563757577/xyzzy
-=-=-=-=-=-=-=-=-=-=-=-