From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14505 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Support SIGEV_THREAD_ID Date: Mon, 5 Aug 2019 22:06:32 -0400 Message-ID: <20190806020632.GS9017@brightrain.aerifal.cx> References: <20190801164958.GH9017@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="251714"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14521-gllmg-musl=m.gmane.org@lists.openwall.com Tue Aug 06 04:06:48 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1huosB-0013Lr-8f for gllmg-musl@m.gmane.org; Tue, 06 Aug 2019 04:06:47 +0200 Original-Received: (qmail 3756 invoked by uid 550); 6 Aug 2019 02:06:45 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 3734 invoked from network); 6 Aug 2019 02:06:44 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14505 Archived-At: On Thu, Aug 01, 2019 at 02:00:44PM -0400, James Y Knight wrote: > On Thu, Aug 1, 2019 at 12:50 PM Rich Felker wrote: > > > > On Thu, Aug 01, 2019 at 12:15:57PM -0400, James Y Knight wrote: > > > There seems to be some debate in glibc over whether this API should be > > > supported, due to the long-standing debate about "pthread_t" vs > > > "kernel tid" APIs. (And this API uses kernel tids, of course.) > > > > > > One proposal from previous glibc discussion was to add a > > > SIGEV_PTHREAD_ID, which takes a pthread_t, instead of a kernel tid. > > > Nobody has done this yet, and I'd note that if it is done, that is not > > > at all incompatible with also continuing to support SIGEV_THREAD_ID. > > > (Just like both sched_setaffinity and pthread_setaffinity_np exist > > > without issue). > > > > > > Despite that discussion, SIGEV_THREAD_ID functionality does in fact > > > work with glibc -- it provides the SIGEV_THREAD_ID define in its > > > headers, and it defines the same 'struct sigevent' as the kernel does, > > > including a _tid member. The only thing it's missing is the field name > > > "sigev_notify_thread_id" -- so users are simply doing "#define > > > sigev_notify_thread_id _sigev_un._tid" as a workaround (ugh). > > > > > > However, it does _not_ work today with musl, as musl's timer_create > > > function translates the user-facing struct to a separate kernel-facing > > > structure. > > > > > > Given long-standing usage of this feature, and given that potential > > > future directions are additive, not conflicting, ISTM reasonable to > > > just add support for this to musl. > > > > > From 0a0aef759f216444f558f427466b47f38d678052 Mon Sep 17 00:00:00 2001 > > > From: James Y Knight > > > Date: Sun, 30 Jun 2019 21:55:20 -0400 > > > Subject: [PATCH] Add support for SIGEV_THREAD_ID and > sigev_notify_thread_id. > > > > > > This is like SIGEV_SIGNAL, but targeted to a particular thread's > > > tid, rather than the process. > > > --- > > > include/signal.h | 16 +++++++++++++--- > > > src/time/timer_create.c | 8 ++++++-- > > > 2 files changed, 19 insertions(+), 5 deletions(-) > > > > > > diff --git a/include/signal.h b/include/signal.h > > > index 5c48cb83..735e0ac7 100644 > > > --- a/include/signal.h > > > +++ b/include/signal.h > > > @@ -180,14 +180,24 @@ struct sigevent { > > > union sigval sigev_value; > > > int sigev_signo; > > > int sigev_notify; > > > - void (*sigev_notify_function)(union sigval); > > > - pthread_attr_t *sigev_notify_attributes; > > > - char __pad[56-3*sizeof(long)]; > > > + union { > > > + char __pad[64 - 2*sizeof(int) - sizeof(union sigval)]; > > > + pid_t sigev_notify_thread_id; > > > + struct { > > > + void (*sigev_notify_function)(union sigval); > > > + pthread_attr_t *sigev_notify_attributes; > > > + } __sev_thread; > > > + } __sev_fields; > > > }; > > > > > > +#define sigev_notify_thread_id __sev_fields.sigev_notify_thread_id > > > +#define sigev_notify_function > __sev_fields.__sev_thread.sigev_notify_function > > > +#define sigev_notify_attributes > __sev_fields.__sev_thread.sigev_notify_attributes > > > + > > > > I really hate these macro hacks, and have been looking at getting rid > > of the ones we have (using anon unions). We don't mandate C11 to use > > the public headers, but it might make sense to mandate C11 || __GNUC__ > > and stick __extension__ on the struct if __GNUC__ is defined. > > Unfortunately, cparser/firm prior to latest git master does not > > support designated initializers right with anon unions, and GCC 3.x > > doesn't either (yes, there are users of GCC 3 with musl, both > > full-time and just as a bootstrapping-from-plain-C stage for distros). > > So I'm not sure if we can fix this yet or just keep doing the same > > nasty macro hack for now... > > 100% agreed! > > I had actually drafted the email below proposing to get rid of this in > favor of C11 anonymous struct/union. However, after writing the below text, > I then abandoned that idea, because _no_ version of the C++ standard > supports anonymous structs -- although every version has supported > anonymous unions. And I figured that because the musl headers must be > parseable in C++, and given musl's goals of standards adherence, the use of > a widely-supported-yet-nonstandard compiler extension would not be > acceptable, even if effectively every C++ compiler in use supports it. The C++ part is really disappointing, since poor interaction with C++ is one of the main reasons I wanted to eliminate the #defines... :( I think we could probably get by with only anon unions, not anon structs, but I haven't done any detailed analysis. We should probably just put this aside for now, though, since it looks less productive relative to the painfulness than I expected... Rich