[PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Andrey Vagin]

This patch adds a new ptrace request PTRACE_PEEKSIGINFO.

This request is used to retrieve information about a signal with the
specified sequence number. A siginfo_t structure is copied from the child
to location data in the parent.

The low 16 bits of addr contains a sequence number of signal in a queue.
All other bits of addr is used for flags. Currently here is only one
flag PTRACE_PEEK_SHARED for dumping signals from process-wide shared
queue. If this flag is not set, a signal is read from a per-thread
queue.  A result siginfo contains a kernel part of si_code which usually
striped, but it's required for queuing the same siginfo back during
restore of pending signals.

If a signal with the specified sequence number doesn't exist, ptrace
returns ENOENT.

This functionality is required for checkpointing pending signals.

The prototype of this code was developed by Oleg Nesterov.

Cc: Roland McGrath <roland-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: "Michael Kerrisk (man-pages)" <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Signed-off-by: Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
---
 include/uapi/linux/ptrace.h |  9 +++++++
 kernel/ptrace.c             | 64 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+)

diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 022ab18..5d851d5 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -52,6 +52,15 @@
 #define PTRACE_INTERRUPT	0x4207
 #define PTRACE_LISTEN		0x4208
 
+#define PTRACE_PEEKSIGINFO	0x4209
+
+/*
+ * The lower 16 bits of addr is a sequence number of a signal.
+ * All other bits can be used for flags.
+ */
+#define PTRACE_PEEKSIGINFO_FLAGS_MASK	(~0UL << 16)
+#define PTRACE_PEEK_SHARED		(1UL << 31)
+
 /* Wait extended result codes for the above trace options.  */
 #define PTRACE_EVENT_FORK	1
 #define PTRACE_EVENT_VFORK	2
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 1599157..27fd31a 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -579,6 +579,40 @@ static int ptrace_setsiginfo(struct task_struct *child, const siginfo_t *info)
 	return error;
 }
 
+#ifdef CONFIG_CHECKPOINT_RESTORE
+static int ptrace_peek_siginfo(struct task_struct *child,
+				unsigned long addr, siginfo_t *siginfo)
+{
+	struct sigpending *pending;
+	struct sigqueue *q;
+	unsigned long flags;
+	unsigned int nr;
+	int ret = -ENOENT;
+
+	nr = addr & ~PTRACE_PEEKSIGINFO_FLAGS_MASK;
+	flags = addr & PTRACE_PEEKSIGINFO_FLAGS_MASK;
+
+	if (flags & PTRACE_PEEK_SHARED)
+		pending = &child->signal->shared_pending;
+	else
+		pending = &child->pending;
+
+	if (flags & ~PTRACE_PEEK_SHARED)
+		return -EINVAL; /* unknown flags */
+
+	spin_lock_irq(&child->sighand->siglock);
+	list_for_each_entry(q, &pending->list, list) {
+		if (!nr--) {
+			copy_siginfo(siginfo, &q->info);
+			ret = 0;
+			break;
+		}
+	}
+	spin_unlock_irq(&child->sighand->siglock);
+
+	return ret;
+}
+#endif
 
 #ifdef PTRACE_SINGLESTEP
 #define is_singlestep(request)		((request) == PTRACE_SINGLESTEP)
@@ -703,6 +737,21 @@ int ptrace_request(struct task_struct *child, long request,
 		ret = put_user(child->ptrace_message, datalp);
 		break;
 
+#ifdef CONFIG_CHECKPOINT_RESTORE
+	case PTRACE_PEEKSIGINFO:
+	{
+		siginfo_t __user *uinfo = (siginfo_t __user *) data;
+
+		ret = ptrace_peek_siginfo(child, addr, &siginfo);
+
+		if (!ret)
+			ret = copy_siginfo_to_user(uinfo, &siginfo);
+		if (!ret)
+			ret = __put_user(siginfo.si_code, &uinfo->si_code);
+		break;
+	}
+#endif
+
 	case PTRACE_GETSIGINFO:
 		ret = ptrace_getsiginfo(child, &siginfo);
 		if (!ret)
@@ -959,6 +1008,21 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
 		ret = put_user((compat_ulong_t) child->ptrace_message, datap);
 		break;
 
+#ifdef CONFIG_CHECKPOINT_RESTORE
+	case PTRACE_PEEKSIGINFO:
+	{
+		compat_siginfo_t __user *uinfo = compat_ptr(data);
+
+		ret = ptrace_peek_siginfo(child, addr, &siginfo);
+
+		if (!ret)
+			ret = copy_siginfo_to_user32(uinfo, &siginfo);
+		if (!ret)
+			ret = __put_user(siginfo.si_code, &uinfo->si_code);
+		break;
+	}
+#endif
+
 	case PTRACE_GETSIGINFO:
 		ret = ptrace_getsiginfo(child, &siginfo);
 		if (!ret)
-- 
1.7.11.7

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Oleg Nesterov]

On 02/13, Andrey Vagin wrote:
>
> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
>
> This request is used to retrieve information about a signal with the
> specified sequence number. A siginfo_t structure is copied from the child
> to location data in the parent.
>
> The low 16 bits of addr contains a sequence number of signal in a queue.
> All other bits of addr is used for flags. Currently here is only one
> flag PTRACE_PEEK_SHARED for dumping signals from process-wide shared
> queue. If this flag is not set, a signal is read from a per-thread
> queue.  A result siginfo contains a kernel part of si_code which usually
> striped, but it's required for queuing the same siginfo back during
> restore of pending signals.
>
> If a signal with the specified sequence number doesn't exist, ptrace
> returns ENOENT.
>
> This functionality is required for checkpointing pending signals.

I thinks the patch is correct, and personally I like very much the fact
it is simple. Compared to misc signalfd hacks we discussed before.

Oleg.

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pedro Alves]

On 02/13/2013 03:16 PM, Andrey Vagin wrote:
> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
> 
> This request is used to retrieve information about a signal with the
> specified sequence number. A siginfo_t structure is copied from the child
> to location data in the parent.
> 
> The low 16 bits of addr contains a sequence number of signal in a queue.
> All other bits of addr is used for flags. Currently here is only one
> flag PTRACE_PEEK_SHARED for dumping signals from process-wide shared
> queue. If this flag is not set, a signal is read from a per-thread
> queue.  A result siginfo contains a kernel part of si_code which usually
> striped, but it's required for queuing the same siginfo back during
> restore of pending signals.
> 
> If a signal with the specified sequence number doesn't exist, ptrace
> returns ENOENT.
> 
> This functionality is required for checkpointing pending signals.
> 
> The prototype of this code was developed by Oleg Nesterov.

Not sure I'm reading the patch right, but it looks like GDB would
be able to use this as alternative to PTRACE_GET_SIGINFO variant
that returns the siginfo_t object in the architecture/bitness of
the tracee, rather than the architecture of the kernel, right?
So it no longer would need to try and replicate the kernel's
siginfo conversions.  I wouldn't mind if this was added unconditionally
instead of wrapped on CONFIG_CHECKPOINT_RESTORE.  We'd miss the poke
variant, but that looks like something that could be always be added
later.

-- 
Pedro Alves

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Oleg Nesterov]

On 02/15, Pedro Alves wrote:
>
> Not sure I'm reading the patch right, but it looks like GDB would
> be able to use this as alternative to PTRACE_GET_SIGINFO variant

No, it is different. PTRACE_GETSIGINFO reports the siginfo for the signal
which was already dequeued (ignoring the fact ->last_siginfo != NULL doesn't
necessarily mean we report a signal), while this patch allows to look at
the pending signals which were not reported yet.

> I wouldn't mind if this was added unconditionally
> instead of wrapped on CONFIG_CHECKPOINT_RESTORE.

I agree. If you think gdb can use this new feature,  CONFIG_ can go away.

> We'd miss the poke
> variant, but that looks like something that could be always be added
> later.

Yes. _POKE_ or _QUEUE_ or _DEQUEUE_, we can add more features if user-
space wants them.

Oleg.

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pedro Alves]

On 02/15/2013 07:43 PM, Oleg Nesterov wrote:
> On 02/15, Pedro Alves wrote:
>>
>> Not sure I'm reading the patch right, but it looks like GDB would
>> be able to use this as alternative to PTRACE_GET_SIGINFO variant
> 
> No, it is different. PTRACE_GETSIGINFO reports the siginfo for the signal
> which was already dequeued (ignoring the fact ->last_siginfo != NULL doesn't
> necessarily mean we report a signal), while this patch allows to look at
> the pending signals which were not reported yet.

Ah, I had assumed queue position 0 would be the dequeued signal.

>> I wouldn't mind if this was added unconditionally
>> instead of wrapped on CONFIG_CHECKPOINT_RESTORE.
> 
> I agree. If you think gdb can use this new feature,  CONFIG_ can go away.

I think so -- we can add a gdb command to dump the list of
pending signals, which I think is something that'd be useful.

> 
>> We'd miss the poke
>> variant, but that looks like something that could be always be added
>> later.
> 
> Yes. _POKE_ or _QUEUE_ or _DEQUEUE_, we can add more features if user-
> space wants them.

-- 
Pedro Alves

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pedro Alves]

Forgot to reply to this bit:

On 02/15/2013 07:43 PM, Oleg Nesterov wrote:
>> We'd miss the poke
>> > variant, but that looks like something that could be always be added
>> > later.
> Yes. _POKE_ or _QUEUE_ or _DEQUEUE_, we can add more features if user-
> space wants them.

In general, IMO, I agree with Roland at https://lkml.org/lkml/2002/12/20/84
in that it's good to have setters for completeness, so that you can
change all the state via ptrace that you can read via ptrace.

But I'm not doing any of this work, hence my "could always be
added later" comment instead of actually requesting it.  But if
we had it, we could make e.g., gdb inspect the signal queues,
and then be able to tweak a realtime signal before it is
delivered.

-- 
Pedro Alves

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Andrey Wagin]

2013/2/16 Pedro Alves <palves-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> Forgot to reply to this bit:
>
> On 02/15/2013 07:43 PM, Oleg Nesterov wrote:
>>> We'd miss the poke
>>> > variant, but that looks like something that could be always be added
>>> > later.
>> Yes. _POKE_ or _QUEUE_ or _DEQUEUE_, we can add more features if user-
>> space wants them.
>
> In general, IMO, I agree with Roland at https://lkml.org/lkml/2002/12/20/84
> in that it's good to have setters for completeness, so that you can
> change all the state via ptrace that you can read via ptrace.
>
> But I'm not doing any of this work, hence my "could always be
> added later" comment instead of actually requesting it.  But if
> we had it, we could make e.g., gdb inspect the signal queues,
> and then be able to tweak a realtime signal before it is
> delivered.

PTRACE_POKESIGINFO is more complicated than PTRACE_PEEKSIGINFO.
Looks like PTRACE_POKESIGINFO should replace a siginfo with the
specified sequence number. Should it be able to change signo? If it is
able, what to do with signalfd, which already got a notification about
the previous signal?..

My opinion is that it "could always be added later", when we will
understand what exactly we want to have.

>
> --
> Pedro Alves

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Michael Kerrisk (man-pages)]

On Wed, Feb 13, 2013 at 4:16 PM, Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> wrote:
> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
>
> This request is used to retrieve information about a signal with the
> specified sequence number. A siginfo_t structure is copied from the child
> to location data in the parent.
>
> The low 16 bits of addr contains a sequence number of signal in a queue:

I think 16 bits is probably not enough.... Already, on the "out of the
box" system that I have at hand, one can queue more than 2^16-1
signals:

    $ cat /proc/$$/status | grep SigQ
    SigQ:	2/126065

Cheers,

Michael


> All other bits of addr is used for flags. Currently here is only one
> flag PTRACE_PEEK_SHARED for dumping signals from process-wide shared
> queue. If this flag is not set, a signal is read from a per-thread
> queue.  A result siginfo contains a kernel part of si_code which usually
> striped, but it's required for queuing the same siginfo back during
> restore of pending signals.
>
> If a signal with the specified sequence number doesn't exist, ptrace
> returns ENOENT.
>
> This functionality is required for checkpointing pending signals.
>
> The prototype of this code was developed by Oleg Nesterov.
>
> Cc: Roland McGrath <roland-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Cc: "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> Cc: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Dave Jones <davej-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: "Michael Kerrisk (man-pages)" <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Pavel Emelyanov <xemul-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
> Cc: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
> Signed-off-by: Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org>
> ---
>  include/uapi/linux/ptrace.h |  9 +++++++
>  kernel/ptrace.c             | 64 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+)
>
> diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
> index 022ab18..5d851d5 100644
> --- a/include/uapi/linux/ptrace.h
> +++ b/include/uapi/linux/ptrace.h
> @@ -52,6 +52,15 @@
>  #define PTRACE_INTERRUPT       0x4207
>  #define PTRACE_LISTEN          0x4208
>
> +#define PTRACE_PEEKSIGINFO     0x4209
> +
> +/*
> + * The lower 16 bits of addr is a sequence number of a signal.
> + * All other bits can be used for flags.
> + */
> +#define PTRACE_PEEKSIGINFO_FLAGS_MASK  (~0UL << 16)
> +#define PTRACE_PEEK_SHARED             (1UL << 31)
> +
>  /* Wait extended result codes for the above trace options.  */
>  #define PTRACE_EVENT_FORK      1
>  #define PTRACE_EVENT_VFORK     2
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index 1599157..27fd31a 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -579,6 +579,40 @@ static int ptrace_setsiginfo(struct task_struct *child, const siginfo_t *info)
>         return error;
>  }
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +static int ptrace_peek_siginfo(struct task_struct *child,
> +                               unsigned long addr, siginfo_t *siginfo)
> +{
> +       struct sigpending *pending;
> +       struct sigqueue *q;
> +       unsigned long flags;
> +       unsigned int nr;
> +       int ret = -ENOENT;
> +
> +       nr = addr & ~PTRACE_PEEKSIGINFO_FLAGS_MASK;
> +       flags = addr & PTRACE_PEEKSIGINFO_FLAGS_MASK;
> +
> +       if (flags & PTRACE_PEEK_SHARED)
> +               pending = &child->signal->shared_pending;
> +       else
> +               pending = &child->pending;
> +
> +       if (flags & ~PTRACE_PEEK_SHARED)
> +               return -EINVAL; /* unknown flags */
> +
> +       spin_lock_irq(&child->sighand->siglock);
> +       list_for_each_entry(q, &pending->list, list) {
> +               if (!nr--) {
> +                       copy_siginfo(siginfo, &q->info);
> +                       ret = 0;
> +                       break;
> +               }
> +       }
> +       spin_unlock_irq(&child->sighand->siglock);
> +
> +       return ret;
> +}
> +#endif
>
>  #ifdef PTRACE_SINGLESTEP
>  #define is_singlestep(request)         ((request) == PTRACE_SINGLESTEP)
> @@ -703,6 +737,21 @@ int ptrace_request(struct task_struct *child, long request,
>                 ret = put_user(child->ptrace_message, datalp);
>                 break;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +       case PTRACE_PEEKSIGINFO:
> +       {
> +               siginfo_t __user *uinfo = (siginfo_t __user *) data;
> +
> +               ret = ptrace_peek_siginfo(child, addr, &siginfo);
> +
> +               if (!ret)
> +                       ret = copy_siginfo_to_user(uinfo, &siginfo);
> +               if (!ret)
> +                       ret = __put_user(siginfo.si_code, &uinfo->si_code);
> +               break;
> +       }
> +#endif
> +
>         case PTRACE_GETSIGINFO:
>                 ret = ptrace_getsiginfo(child, &siginfo);
>                 if (!ret)
> @@ -959,6 +1008,21 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
>                 ret = put_user((compat_ulong_t) child->ptrace_message, datap);
>                 break;
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +       case PTRACE_PEEKSIGINFO:
> +       {
> +               compat_siginfo_t __user *uinfo = compat_ptr(data);
> +
> +               ret = ptrace_peek_siginfo(child, addr, &siginfo);
> +
> +               if (!ret)
> +                       ret = copy_siginfo_to_user32(uinfo, &siginfo);
> +               if (!ret)
> +                       ret = __put_user(siginfo.si_code, &uinfo->si_code);
> +               break;
> +       }
> +#endif
> +
>         case PTRACE_GETSIGINFO:
>                 ret = ptrace_getsiginfo(child, &siginfo);
>                 if (!ret)
> --
> 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/

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pavel Emelyanov]

On 02/19/2013 04:15 PM, Michael Kerrisk (man-pages) wrote:
> On Wed, Feb 13, 2013 at 4:16 PM, Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> wrote:
>> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
>>
>> This request is used to retrieve information about a signal with the
>> specified sequence number. A siginfo_t structure is copied from the child
>> to location data in the parent.
>>
>> The low 16 bits of addr contains a sequence number of signal in a queue:
> 
> I think 16 bits is probably not enough.... Already, on the "out of the
> box" system that I have at hand, one can queue more than 2^16-1
> signals:
> 
>     $ cat /proc/$$/status | grep SigQ
>     SigQ:	2/126065

Yup :( Well, actually it would be enough to have only 1 bit as "flags"
and the rest (31 at least) for the number. But taking into account
Oleg's wish to have an ability to extend the amount of flags in the
future, we should make addr point to something like

struct siginfo_peek_options {
	unsigned int flags;
	unsigned int pos;
};

and force flags to contain zero in all the bits but one, and this
latter bit is to select between private or shared queue. In this case
the patch loses its main advantage -- the simplicity.

Oleg, please, suggest which way would be more preferable?

> Cheers,
> 
> Michael

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Oleg Nesterov]

On 02/19, Pavel Emelyanov wrote:
>
> On 02/19/2013 04:15 PM, Michael Kerrisk (man-pages) wrote:
> > On Wed, Feb 13, 2013 at 4:16 PM, Andrey Vagin <avagin@openvz.org> wrote:
> >> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
> >>
> >> This request is used to retrieve information about a signal with the
> >> specified sequence number. A siginfo_t structure is copied from the child
> >> to location data in the parent.
> >>
> >> The low 16 bits of addr contains a sequence number of signal in a queue:
> >
> > I think 16 bits is probably not enough.... Already, on the "out of the
> > box" system that I have at hand, one can queue more than 2^16-1
> > signals:
> >
> >     $ cat /proc/$$/status | grep SigQ
> >     SigQ:	2/126065
>
> Yup :( Well, actually it would be enough to have only 1 bit as "flags"
> and the rest (31 at least) for the number. But taking into account
> Oleg's wish to have an ability to extend the amount of flags

I am not sure this is really needed, and we can add more PTRACE_PEEK_
codes later. I am fine either way, we can even add PEEK_PRIVATE/SHARED
right now.

But, given that every PEEK does list_for_each() until it finds the
necessary sequence number, I am wondering how this O(n**2) will work
if you want to dump 126065 signals ;)

Oleg.

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pavel Emelyanov]

On 02/19/2013 11:34 PM, Oleg Nesterov wrote:
> On 02/19, Pavel Emelyanov wrote:
>>
>> On 02/19/2013 04:15 PM, Michael Kerrisk (man-pages) wrote:
>>> On Wed, Feb 13, 2013 at 4:16 PM, Andrey Vagin <avagin-GEFAQzZX7r8dnm+yROfE0A@public.gmane.org> wrote:
>>>> This patch adds a new ptrace request PTRACE_PEEKSIGINFO.
>>>>
>>>> This request is used to retrieve information about a signal with the
>>>> specified sequence number. A siginfo_t structure is copied from the child
>>>> to location data in the parent.
>>>>
>>>> The low 16 bits of addr contains a sequence number of signal in a queue:
>>>
>>> I think 16 bits is probably not enough.... Already, on the "out of the
>>> box" system that I have at hand, one can queue more than 2^16-1
>>> signals:
>>>
>>>     $ cat /proc/$$/status | grep SigQ
>>>     SigQ:	2/126065
>>
>> Yup :( Well, actually it would be enough to have only 1 bit as "flags"
>> and the rest (31 at least) for the number. But taking into account
>> Oleg's wish to have an ability to extend the amount of flags
> 
> I am not sure this is really needed, and we can add more PTRACE_PEEK_
> codes later. I am fine either way, we can even add PEEK_PRIVATE/SHARED
> right now.
> 
> But, given that every PEEK does list_for_each() until it finds the
> necessary sequence number, I am wondering how this O(n**2) will work
> if you want to dump 126065 signals ;)

Isn't it the great reason for making the addr point to a structure, that
would look like

struct siginfo_peek_arg {
	unsigned flags; /* all bits but 0th, that selects between private/shared
			   queues, should be zero */
	unsigned int off; /* from which siginfo to start */
	unsigned int nr; /* how may siginfos to take */
};

? :)

> Oleg.
> 
> .

Thanks,
Pavel

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Oleg Nesterov]

On 02/19, Pavel Emelyanov wrote:
>
> On 02/19/2013 11:34 PM, Oleg Nesterov wrote:
> > But, given that every PEEK does list_for_each() until it finds the
> > necessary sequence number, I am wondering how this O(n**2) will work
> > if you want to dump 126065 signals ;)
>
> Isn't it the great reason for making the addr point to a structure, that
> would look like
>
> struct siginfo_peek_arg {
> 	unsigned flags; /* all bits but 0th, that selects between private/shared
> 			   queues, should be zero */
> 	unsigned int off; /* from which siginfo to start */
> 	unsigned int nr; /* how may siginfos to take */
> };

I am fine either way, to me everything looks better than signalfd
hacks.

But if you meant "avoid n^2", this won't help? You can't do
copy_siginfo_to_user() under ->siglock, so you need to restart
list_for_each() anyway.

Oleg.

Re: [PATCH] ptrace: add ability to retrieve signals without removing them from a queue

[Pavel Emelyanov]

On 02/21/2013 02:37 AM, Oleg Nesterov wrote:
> On 02/19, Pavel Emelyanov wrote:
>>
>> On 02/19/2013 11:34 PM, Oleg Nesterov wrote:
>>> But, given that every PEEK does list_for_each() until it finds the
>>> necessary sequence number, I am wondering how this O(n**2) will work
>>> if you want to dump 126065 signals ;)
>>
>> Isn't it the great reason for making the addr point to a structure, that
>> would look like
>>
>> struct siginfo_peek_arg {
>> 	unsigned flags; /* all bits but 0th, that selects between private/shared
>> 			   queues, should be zero */
>> 	unsigned int off; /* from which siginfo to start */
>> 	unsigned int nr; /* how may siginfos to take */
>> };
> 
> I am fine either way, to me everything looks better than signalfd
> hacks.
> 
> But if you meant "avoid n^2", this won't help? You can't do
> copy_siginfo_to_user() under ->siglock, so you need to restart
> list_for_each() anyway.

Or allocate intermediate buffer putting the siginfo's there.

> Oleg.

Thanks,
Pavel