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