From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754033Ab3BJKFq (ORCPT ); Sun, 10 Feb 2013 05:05:46 -0500 Received: from relay.parallels.com ([195.214.232.42]:51971 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751480Ab3BJKFn (ORCPT ); Sun, 10 Feb 2013 05:05:43 -0500 Date: Sun, 10 Feb 2013 14:04:24 +0400 From: Andrew Vagin To: "Michael Kerrisk (man-pages)" CC: Oleg Nesterov , David Howells , Pavel Emelyanov , , , , Cyrill Gorcunov , Andrey Wagin , Alexander Viro , , Dave Jones , "Paul E. McKenney" Subject: Re: [CRIU] [PATCH 3/3] signalfd: add ability to read siginfo-s without dequeuing signals (v2) Message-ID: <20130210100424.GA15978@paralelels.com> References: <1358849741-9611-4-git-send-email-avagin@openvz.org> <1359486181-29088-1-git-send-email-avagin@openvz.org> <20130207173447.GA5888@redhat.com> <20130208191056.GA13674@redhat.com> <20130209182239.GA9947@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset="koi8-r" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Originating-IP: [79.172.101.205] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Feb 09, 2013 at 11:53:04PM +0100, Michael Kerrisk (man-pages) wrote: > On Sat, Feb 9, 2013 at 7:22 PM, Oleg Nesterov wrote: > > On 02/08, Michael Kerrisk (man-pages) wrote: > >> > >> On Fri, Feb 8, 2013 at 8:10 PM, Oleg Nesterov 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? > siginfo_t *signals = malloc(...); ... ret = ptrace(PTRACE_ATTACH, pid, NULL, NULL); if (ret == -1) goto err; for (i = 0; ; i++) { ret = ptrace(PTRACE_PEEKSIGNAL, pid, i, signals + i); if (ret == -1) { if (errno == ESOMETHING) break; else goto err_detach; } } ... err_detach: ret = ptrace(PTRACE_DETACH, pid, NULL, NULL); err: ... For shared (per-process) signals one line should be changed: ret = ptrace(PTRACE_PEEKSIGNAL, pid, PTRACE_PEEK_SHARED + i, signals + i); > 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/ > _______________________________________________ > CRIU mailing list > CRIU@openvz.org > https://lists.openvz.org/mailman/listinfo/criu