Re: [CRIU] [PATCH 3/3] signalfd: add ability to read siginfo-swithout dequeuing signals (v2)

From: Andrew Vagin
Date: Mon Feb 11 2013 - 06:01:13 EST


On Mon, Feb 11, 2013 at 10:29:50AM +0100, Denys Vlasenko wrote:
> On Friday 08 February 2013 21:15, Michael Kerrisk (man-pages) wrote:
> > >> >Damn. But after I wrote this email I realized that llseek() probably can't
> > >> > work. Because peek_offset/f_pos/whatever has to be shared with all processes
> > >> > which have this file opened.
> > >
> > > Yes. but I thought you decided to ignore this oddity ;)
> > >
> > >> So I want to suggest a way how to forbid read() for SIGNALFD_PEEK.
> > >> file->f_pos can be initialized to -1. read() returns EINVAL in this
> > >> case. In a man page we will write that signals can be dumped only with
> > >> help pread(). Is it overload or too ugly?
> > >
> > > Well. I do not know. Up to you and Michael.
> > >
> > > But honestly, I can't say this all looks really nice. And why do we
> > > need SIGNALFD_PEEK then?
> >
> > It surely is no beauty. The hope is at least to make it less ugly than it was.
> >
> > > Seriously, perhaps we should simply add signalfd_fops->ioctl() for PEEK.
> > > Or add PTRACE_{PEEK,POKE}_SIGNAL which looks even logical and useful...
> > > And much simpler/straightforward.
> > >
> > > But I am not going to argue.
>
> ptrace interface might find some use in debuggers.
> Not that any of them expressed such desires so far,
> but just maybe.
>
> However, it needs coding in C to read it,
> which brings me to:
>
>
> > I suppose I had wondered along similar lines, but in a slightly
> > different direction: would the use of a /proc interface to get the
> > queued signals make some sense?
>
> I think that /proc interface beats adding magic flags and magic semantic
> to [p]read.
>
> It also has the benefit of being human-readable. You don't need
> to write a special C program to "cat /proc/$$/foo".
>
> Andrey, I know that it is hard to let go of the code you invested time
> and efforts in creating. But this isn't the last patch, is it?
> You will need to retrieve yet more data for process checkpointing.
> When you start working on the next patch for it, consider trying
> /proc approach.

I don't think that we need to convert siginfo into a human readable format
in kernel. For example siginfo with a negative si_code contains binary data.
A kernel already has got signalfd_siginfo and we can see, that it's not
good.
I think would be better to print siginfo in a human readable format from
userspace (e.g. gdb) and implement a generic interface in kernel (e.g.
ptrace).

> > --
> vda
> _______________________________________________
> CRIU mailing list
> CRIU@xxxxxxxxxx
> https://lists.openvz.org/mailman/listinfo/criu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/