[PATCH] V1 1/2] ipc: let message queues use SIGEV_THREAD_ID with mq_notify

[PATCH] V1 1/2] ipc: let message queues use SIGEV_THREAD_ID with mq_notify

[Steven Stewart-Gallus]

From: Steven Stewart-Gallus <sstewartgallus00@mylangara.bc.ca>

Currently the only thread-safe way of using mq_notify with message
queues is to use the SIGEV_THREAD option. Unfortunately, existing
wrappers around such functionality spawn a thread each time a
notification happens instead of caching threads which is slow and
inconvenient for debugging. This change lets message queues use
SIGEV_THREAD_ID with mq_notify and so target notifications to only one
thread at a time which is thread-safe.

Signed-off-by: Steven Stewart-Gallus <sstewartgallus00@mylangara.bc.ca>
---

Hello! This is a simple patch I made more for the purpose of learning
how the kernel works then for any serious purpose. Currently, in my
own program I now simply use a thread pool and poll for asynchronously
running message sends and receives but a thread-safe mq_notify would
have been useful for me and possibly saved me some effort. So, I won't
be seriously bothered if this patch is rejected but I think other
people might find it useful nonetheless. I also think this behaviour
seems quite natural and appropriate to support.

I am sending this patch for review but I am pretty sure it is mistaken
in some ways. In particular, I believe this patch is wrong because I
don't believe it filters out thread ids that belong to other processes
and I am not sure how to do that.

Thank you,
Steven Stewart-Gallus

diff --git a/man3/mq_notify.3 b/man3/mq_notify.3
index a71aac4..41a1b96 100644
--- a/man3/mq_notify.3
+++ b/man3/mq_notify.3
@@ -95,6 +95,16 @@ as if it were the start function of a new thread.
 See
 .BR sigevent (7)
 for details.
+.TP
+.BR SIGEV_THREAD_ID " (Linux-specific)"
+As for
+.BR SIGEV_SIGNAL ,
+but the signal is targeted at the thread whose ID is given in
+.IR sigev_notify_thread_id ,
+which must be a thread in the same process as the caller.
+See
+.BR sigevent (7)
+for general details.
 .PP
 Only one process can be registered to receive notification
 from a message queue.
diff --git a/man7/sigevent.7 b/man7/sigevent.7
index 9cec77e..15da9fe 100644
--- a/man7/sigevent.7
+++ b/man7/sigevent.7
@@ -125,8 +125,10 @@ structure that defines attributes for the new thread (see
 .TP
 .BR SIGEV_THREAD_ID " (Linux-specific)"
 .\" | SIGEV_SIGNAL vs not?
-Currently used only by POSIX timers; see
-.BR timer_create (2).
+Currently used only by POSIX timers and message queues; see
+.BR timer_create (2)
+and
+.BR mq_notify (2).
 .SH CONFORMING TO
 POSIX.1-2001.
 .SH SEE ALSO

Re: [PATCH] V1 1/2] ipc: let message queues use SIGEV_THREAD_ID with mq_notify

[Manfred Spraul]

Hi Steven,

You wrote:
> Currently the only thread-safe way of using mq_notify with message 
> queues is to use the SIGEV_THREAD option.
Could you explain what you mean with "only thread-safe way"?
I'm a bit relunctant to extend mq_notify() without understanding the reason.

What about:
- use sigprocmask()
- create one worker thread
- then in a loop in that worker thread: use sigwaitinfo() or signalfd() 
to collect the signals.


And one point I don't like: Within timer_create():
>  SIGEV_THREAD_ID (Linux-specific)
>  [...]
> The sigev_notify_thread_id field specifies a kernel thread ID, that 
> is, the value returned by clone(2) or gettid(2).
Does that mean that SIGEV_THREAD_ID is guaranteed to remain 
Linux-specific, it's implicitely linked to the Linux clone()/gettid() 
threading model?

> This flag is intended only for use by threading libraries.


--
     Manfred

Re: [PATCH] V1 1/2] ipc: let message queues use SIGEV_THREAD_ID with mq_notify

[Steven Stewart-Gallus]

> Could you explain what you mean with "only thread-safe way"?
> I'm a bit relunctant to extend mq_notify() without understanding 
> the reason.
> 
> What about:
> - use sigprocmask()
> - create one worker thread
> - then in a loop in that worker thread: use sigwaitinfo() or 
> signalfd() 
> to collect the signals.

Those all modify global state and are unusable by library
code. Hogging a signal for your own library is bad.

> And one point I don't like: Within timer_create():

> >  SIGEV_THREAD_ID (Linux-specific)
> >  [...]
> > The sigev_notify_thread_id field specifies a kernel thread ID,
> > that is, the value returned by clone(2) or gettid(2).
>
> Does that mean that SIGEV_THREAD_ID is guaranteed to remain
> Linux-specific, it's implicitely linked to the Linux
> clone()/gettid() threading model?
>
> > This flag is intended only for use by threading libraries.

Yes this is low-level Linux specific stuff.

Maybe GLibc could create a more abstract and portable interface on top
of SIGEV_THREAD_ID that other OS's and libraries could implement? This
is pretty much not relevant to Linux kernel code though except that
more features that use SIGEV_THREAD_ID would motivate GLibc developers
to create a wrapper over this and that maybe the Linux kernel could be
modified to make it easier to create a more abstract and portable
wrapper.

If you feel that the whole SIGEV_THREAD_ID line of functionality was a
mistake in the first place and that people should use signals less and
use functionality like epoll more that's also okay.

If you also feel that nobody would use the feature in the first place
that's okay too.

Personally, it just bugged me that SIGEV_THREAD_ID was only used by
timers and not in a few other places where it seems like an obvious
fit.

Thank you,
Steven Stewart-Gallus