Re: [PATCH 0/3] signalfd: a kernel interface for dumping pending signals

[Andrew Vagin <avagin@parallels.com>]

On Wed, Jan 23, 2013 at 05:19:24AM +0100, Michael Kerrisk (man-pages) wrote:
> Hi Andrey,
> 
> On Tue, Jan 22, 2013 at 11:15 AM, Andrey Vagin <avagin@openvz.org> wrote:
> > This patch set adds ability to choose a signal queue and
> > to read signals without dequeuing them.
> >
> > Three new flags are added:
> > SFD_SHARED_QUEUE     -- reads will be from process-wide shared signal queue
> > SFD_PER_THREAD_QUEUE -- reads will be from per-thread signal queue
> > SFD_PEEK             -- don't dequeue signals

> 
> A fuller description of the patch, including information that was in
> previous versions of this patch would be helpful. Let me see if I can
> summarize/fill out the API side of things, and ask a few questions
> along the way (yes, I could answer some of the questions by checking
> the code, but I want to know what the *intended* behavior is).
> 
> The patch series adds a total of 4 flags to signalfd(). In addition to
> those you list above, the other is

In additional we can say, that this patch series adds three orthogonal,
independent groups of flags.
* SFD_RAW
* SFD_PEEK
* SFD_SHARED_QUEUE, SFD_PER_THREAD_QUEUE

> 
> SFD_RAW -- return raw siginfo structs when reading, rather than signalfd_siginfo
> 
> The intention is that these flags be used in conjunction with pread(),
> to peek at queued signals. The 'offset' argument is treated as a
> position. Thus, for example, to non-destructively read all of the
> per-thread signals in raw form from the per-thread queue, one would
> write
> 

siginfo_t *buf;

> fd = signalfd(-1, SFD_PER_THREAD_QUEUE | SFD_RAW | SFD_PEEK)
> for (j = 0; ; j++) {
>     s = pread(fd, buf, ocunt, j)
      s = pread(fd, buf + j, sizeof(siginfo_t), j);
>     if (s <= 0) /* No more signals */
>         break;
> }

This examples reads signals one by one

or

  siginfo_t *buf = NULL;
  unsigned long buf_size = 0, nr = 0;
  int ret;

  while (1) {
	bug_size += PAGE_SIZ;
	buf = realloc(buf, buf_size);
	if (buf == NULL)
		goto err;
	ret = pread(fd, buf + nr, sizeof(siginfo_t), nr);
	if (ret == -1)
		goto err;
	nr += ret / sizeof(siginfo_t);
	if (ret < PAGE_SIZE) /* No more signals */
		break;
  }

pread() can read more than one signal.

* The interface of signalfd could be a bit more predictable,
  if we will treat pos as offset in bytes, not in elements.

  pread(fd, buf, sizeof(siginfo_t), i * sizeof(siginfo_t)) -
               reads a signal with a sequence number i in a queue.
> 
> Right?
> 
> Now some questions. I don't require all of the following, but I'm
> wanting to know what's possible, for documentation purposes.
> 
> Q1: with this patch series, is it permissible to specify
> SFD_PER_THREAD_QUEUE or SFD_SHARED_QUEUE without specifying either
> SFD_PEEK or SFD_QUEUE? In other words, can one do traditional
> signalfd_siginfo reads, but selecting from a specific queue.

Yes, we can
> 
> Q2: Is it possible to specify SFD_PEEK without SFD_RAW, so that one
> can peek at siginfo structs rather than signalfd_siginfo structs?

Yes, it is possible. read() and pread() returns signalfd_siginfo structs
in this case.

> 
> Q3: Is it possible to specify SFD_RAW without SFD_PEEK, so that one
> can destructively read signalfd_siginfo structs? Can that be done
> using any read interface (read(), pread(), etc.)?
Yes, it is possible too. read() will return siginfo structs.

> 
> Q4: Is it possible to specify both SFD_PER_THREAD_QUEUE and
> SFD_SHARED_QUEUE? In that case, in what order are signals read from
> the two queues?
> 

It is equal to the case, when none of these flags are not specified.
And it is equal to what we had before this patches.
signalfd() reads signals from a private queue, then from a shared queue.

ps: A good analogy:
signalfd looks like a pipe if SFD_PEEK are not specified, otherwise it looks
like a file

> Thanks,
> 
> Michael
> 
> > Cc: Serge Hallyn <serge.hallyn@canonical.com>
> > Cc: Oleg Nesterov <oleg@redhat.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: Pavel Emelyanov <xemul@parallels.com>
> > CC: Cyrill Gorcunov <gorcunov@openvz.org>
> > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> >
> > --
> > 1.7.11.7
> >
> 
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Author of "The Linux Programming Interface"; http://man7.org/tlpi/