Re: pcp updates: libpcp locking and concurrency control changes

Thanks for the on-going dialogue Frank.

On 17/01/17 01:36, Frank Ch. Eigler wrote:
> ...
> Or more alternately, we can consider exit() as .safe rather than
> .unsafe, but ...

Fair call.  I tossed a coin on this one ... Plan A was to treat exit() as unsafe and mark calls in source, Plan B was to treat exit() as "we don't care about a race on the exit code" and leave source alone.

First time, Plan A won.  But on reflection, Plan B is better, so I'll make that change.

> ...
> These might be world-ending conditions from the point of view of
> particular pcp client -applications- we ship, not from libpcp per se,
> and definitely not for other larger applications for whom libpcp is
> peripheral.  libpcp should return error codes and let the apps decide.
> (See also RHBZ1187588 / commit 616acd0d74.)

This is another topic I'm not going to try and tackle this in the context (sic) of these changes.  So, that set of issues will have to wait I'm afraid.

> That seems really unfortunate - to enshrine libc locking artifacts in
> libpcp abi + api.  I can't think of any precedent in other libraries.

I'm not married to the idea ... if there is no precedent I'll drop the idea.  But without this (weak) justification, I think the basic approach of a single mutex for calls out to potentially unsafe routines is sound, and I plan to stick with that as I move onto to wrestle the larger locking fish into the pan.

> How about this alternative: we autoconf libpcp for the presence of all
> the various thread-safe libc/posix FOO_r functions we really require.
> If they're there, we use (only) them, and thus no _extlock.  If they're
> not there, we configure libpcp as single-threaded, so again no _extlock.

I don't think this is necessary.

Here's the list of where __pmLock_extcall is being used at the moment:

Source		Reason
access.c	hoststrerror 
accounts.c	getgrgid but only if getgrgid_r is not available
		getpwuid but only if getpwuid_r is not available
		getpwnam but only if getpwnam_r is not available
		getgrnam but only if getgrnam_r is not available
		getenv
		setgrent-endgrent block
auxconnect.c	getenv
		hoststrerror
auxserver.c	getenv
avahi.c		getenv
config.c	getenv
connect.c	getenv
connectlocal.c	dlerror
context.c	getenv
		opendir
derive.c	- no call, protecting a one-trip control variable
		opendir
		getenv
err.c		sasl_errstring
		strerror but only if strerror_r is not available
fault.c		getenv
		pmdaCacheOp	et al
getopt.c	setenv
		getenv
interp.c	getenv
lock.c		getenv
logconnect.c	getenv
		hoststrerror
logutil.c	tmpnam but only if mkstemp is not available
pdu.c		getenv
tz.c		getenv
		setenv
util.c		getenv
		opendir
win32.c		RegisterEventSource
		DeregisterEventSource

So we already have the *_r bases pretty well covered and __pmLock_extcall is not used for most platforms because the alternative routines are not used.

What's left is opendir (that will be fixed by higher level mutex guarantees as we've discussed previously), getenv/setenv and other routines that don't have *_r alternates.

I still doubt there is going to be any reduction of concurrency in practice from the use of __pmLock_extcall.




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

You receive all messages sent to this group.

View/Reply Online (#14991): https://groups.io/g/pcp/message/14991
View All Messages In Topic (5): 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
-=-=-=-=-=-=-=-=-=-=-=-