linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Matt Fleming <matt@console-pimps.org>
Cc: Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Matt Fleming <matt.fleming@linux.intel.com>
Subject: Re: [RFC][PATCH 1/5] signals: Always place SIGCONT and SIGSTOP on 'shared_pending'
Date: Tue, 5 Apr 2011 22:19:58 +0200	[thread overview]
Message-ID: <20110405201958.GA1404@redhat.com> (raw)
In-Reply-To: <1302031310-1765-2-git-send-email-matt@console-pimps.org>

Hi Matt,

I'll try to study this series, but not before Friday, sorry.

Only one thing,

On 04/05, Matt Fleming wrote:
> 
> Because SIGCONT and SIGSTOP affect an entire thread group,

Yes, the effect is global, but

> we can
> place them on the 'shared_pending' queue.

I don't think we can.

 -	pending = group ? &t->signal->shared_pending : &t->pending;
> +	/*
> +	 * We always enqueue SIGSTOP or SIGCONT signals on the shared
> +	 * queue. This means that a SIGSTOP or SIGCONT signal _cannot_
> +	 * be present on a thread's private pending queue.
> +	 *
> +	 * This makes prepare_signal() more optimal as we do not have
> +	 * to remove signals from each thread's pending queue and so
> +	 * can avoid iterating over all threads in the thread group
> +	 * (and therefore avoid the locking that would be necessary to
> +	 * do that safely).
> +	 */
> +	if (group || sig_kernel_stop(sig) || sig == SIGCONT)
> +		pending = &t->signal->shared_pending;
> +	else
> +		pending = &t->pending;

How so? Suppose the process has a handler for SIGCONT. Suppose this
process is not stopped. tkill(SIGCONT) should deliver the signal to
the right thread.

SIGSTOP can't have the handler, still we shouldn't place it on the
shared list, debuggers won't be happy.

Also. This code was changed very much, please do these changes on
top of
http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=refs/heads/ptrace

Oleg.


  reply	other threads:[~2011-04-05 20:20 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-05 19:21 [RFC][PATCH 0/5] Improve signal delivery scalability Matt Fleming
2011-04-05 19:21 ` [RFC][PATCH 1/5] signals: Always place SIGCONT and SIGSTOP on 'shared_pending' Matt Fleming
2011-04-05 20:19   ` Oleg Nesterov [this message]
2011-04-05 20:50     ` Matt Fleming
2011-04-06 12:57       ` Oleg Nesterov
2011-04-06 13:09         ` Tejun Heo
2011-04-06 13:30           ` Matt Fleming
2011-04-06 13:15         ` Matt Fleming
2011-04-11 18:50           ` Oleg Nesterov
2011-04-11 19:24             ` Matt Fleming
2011-04-05 19:21 ` [RFC][PATCH 2/5] signals: Introduce per-thread siglock and action rwlock Matt Fleming
2011-04-13 19:42   ` Oleg Nesterov
2011-04-14 10:34     ` Matt Fleming
2011-04-14 19:00       ` Oleg Nesterov
2011-04-16 13:08         ` Matt Fleming
2011-04-18 16:45           ` Oleg Nesterov
2011-04-21 19:03             ` arch/tile/kernel/hardwall.c:do_hardwall_trap unsafe/wrong usage of ->sighand Oleg Nesterov
2011-04-22 13:04               ` Chris Metcalf
2011-04-26 20:36                 ` [PATCH 0/1] tile: do_hardwall_trap: do not play with task->sighand Oleg Nesterov
2011-04-26 20:37                   ` [PATCH 1/1] " Oleg Nesterov
2011-05-02 22:42                     ` Chris Metcalf
2011-04-26  9:46             ` [RFC][PATCH 2/5] signals: Introduce per-thread siglock and action rwlock Matt Fleming
2011-04-05 19:21 ` [RFC][PATCH 3/5] ia64: Catch up with new sighand action spinlock Matt Fleming
2011-04-05 19:21 ` [RFC][PATCH 4/5] signals: Introduce __dequeue_private_signal helper function Matt Fleming
2011-04-05 19:21 ` [RFC][PATCH 5/5] signals: Don't hold shared siglock across signal delivery Matt Fleming
2011-04-13 20:12   ` Oleg Nesterov
2011-04-14 10:57     ` Matt Fleming
2011-04-14 19:20       ` Oleg Nesterov
2011-04-16 13:27         ` Matt Fleming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110405201958.GA1404@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt.fleming@linux.intel.com \
    --cc=matt@console-pimps.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).