On Sat, Feb 9, 2013 at 7:22 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> On 02/08, Michael Kerrisk (man-pages) wrote:
>>
>> On Fri, Feb 8, 2013 at 8:10 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>> >
>> > 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.
>
> This is subjective, but I am not sure about "less" ;) Yes, we avoid the
> magic offsets, but we add SFD_SHARED/PER_THREAD which need to change
> dequeue_signal plus other complications. And for what?
>
>> > 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.
>>
>> 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?
>
> (Can't resist sorry... yes we need /proc/pid/cr or /dev/cr or whatever
> which dumps almost everything c/r needs without need to add a lot of
> cr code everywhere).
>
> Perhaps, but I am not sure about the textual representation.
>
> And to me, the best solution is the simplest solution. Please look
> at the patch below. It is trivial. And we can also drop the SFD_RAW
> patch in -mm.
Oleg,
This looks promising, but I am not sure I understand the user-space
API. Could you explain how it would look to (say) pull all per-thread
signals from user space?
Thanks,
Michael
> --- x/kernel/ptrace.c
> +++ x/kernel/ptrace.c
> @@ -618,6 +618,35 @@ static int ptrace_setsiginfo(struct task
> return error;
> }
>
> +static int ptrace_peek_signal(struct task_struct *child,
> + unsigned long addr, siginfo_t __user *uinfo)
> +{
> + siginfo_t info;
> + struct sigpending *pending;
> + int ret = -ESOMETHING;
> +
> + pending = &child->pending;
> + if (addr & PTRACE_PEEK_SHARED) {
> + addr &= ~PTRACE_PEEK_SHARED;
> + pending = &child->signal->shared_pending;
> + }
> +
> + spin_lock_irq(&child->sighand->siglock);
> + list_for_each_entry(q, &pending->list, list) {
> + if (!addr--) {
> + copy_siginfo(info, &q->info);
> + ret = 0;
> + break;
> + }
> + }
> + spin_lock_irq(&child->sighand->siglock);
> +
> + if (!ret)
> + ret = copy_siginfo_to_user(uinfo, info);
> + if (!ret)
> + ret = __put_user(info, si_code);
> + return ret;
> +}
>
> #ifdef PTRACE_SINGLESTEP
> #define is_singlestep(request) ((request) == PTRACE_SINGLESTEP)
> @@ -742,6 +771,10 @@ int ptrace_request(struct task_struct *c
> ret = put_user(child->ptrace_message, datalp);
> break;
>
> + case PTRACE_PEEKSIGNAL:
> + ret = ptrace_peek_signal(child, addr, datavp);
> + break;
> +
> case PTRACE_GETSIGINFO:
> ret = ptrace_getsiginfo(child, &siginfo);
> if (!ret)
>
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface"; http://man7.org/tlpi/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
