All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: Waiman Long <longman@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	selinux@vger.kernel.org, Paul Moore <paul@paul-moore.com>,
	Stephen Smalley <sds@tycho.nsa.gov>,
	Eric Paris <eparis@parisplace.org>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH 2/4] signal: Make flush_sigqueue() use free_q to release memory
Date: Thu, 21 Mar 2019 18:52:08 -0700	[thread overview]
Message-ID: <20190322015208.GD19508@bombadil.infradead.org> (raw)
In-Reply-To: <20190321214512.11524-3-longman@redhat.com>

On Thu, Mar 21, 2019 at 05:45:10PM -0400, Waiman Long wrote:
> It was found that if a process had many pending signals (e.g. millions),
> the act of exiting that process might cause its parent to have a hard
> lockup especially on a debug kernel with features like KASAN enabled.
> It was because the flush_sigqueue() was called in release_task() with
> tasklist_lock held and irq disabled.

This rather apocalyptic language is a bit uncalled for.  I appreciate the
warning is scary, but all that's really happening is that the debug code
is taking too long to execute.

> To avoid this dire condition and reduce lock hold time of tasklist_lock,
> flush_sigqueue() is modified to pass in a freeing queue pointer so that
> the actual freeing of memory objects can be deferred until after the
> tasklist_lock is released and irq re-enabled.

I think this is a really bad solution.  It looks kind of generic,
but isn't.  It's terribly inefficient, and all it's really doing is
deferring the debugging code until we've re-enabled interrupts.

We'd be much better off just having a list_head in the caller
and list_splice() the queue->list onto that caller.  Then call
__sigqueue_free() for each signal on the queue.

  reply	other threads:[~2019-03-22  1:52 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-21 21:45 [PATCH 0/4] Signal: Fix hard lockup problem in flush_sigqueue() Waiman Long
2019-03-21 21:45 ` [PATCH 1/4] mm: Implement kmem objects freeing queue Waiman Long
2019-03-22 17:47   ` Christopher Lameter
2019-03-22 17:47     ` Christopher Lameter
2019-03-21 21:45 ` [PATCH 2/4] signal: Make flush_sigqueue() use free_q to release memory Waiman Long
2019-03-22  1:52   ` Matthew Wilcox [this message]
2019-03-22 11:16     ` Oleg Nesterov
2019-03-22 16:10       ` Waiman Long
2019-03-22 17:50         ` Christopher Lameter
2019-03-22 17:50           ` Christopher Lameter
2019-03-22 18:12           ` Waiman Long
2019-03-22 19:39             ` Christopher Lameter
2019-03-22 19:39               ` Christopher Lameter
2019-03-22 19:59               ` Matthew Wilcox
2019-03-25 14:15                 ` Christopher Lameter
2019-03-25 14:15                   ` Christopher Lameter
2019-03-25 15:26                   ` Matthew Wilcox
2019-03-25 16:16                     ` Christopher Lameter
2019-03-25 16:16                       ` Christopher Lameter
2019-03-26 13:36                   ` Oleg Nesterov
2019-03-26 13:29           ` Oleg Nesterov
2019-03-21 21:45 ` [PATCH 3/4] signal: Add free_uid_to_q() Waiman Long
2019-03-21 21:45 ` [PATCH 4/4] mm: Do periodic rescheduling when freeing objects in kmem_free_up_q() Waiman Long
2019-03-21 22:00   ` Peter Zijlstra
2019-03-22 14:35     ` Waiman Long
2019-03-22 10:15 ` [PATCH 0/4] Signal: Fix hard lockup problem in flush_sigqueue() Matthew Wilcox
2019-03-22 11:49   ` Oleg Nesterov

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=20190322015208.GD19508@bombadil.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=eparis@parisplace.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=longman@redhat.com \
    --cc=oleg@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=penberg@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rientjes@google.com \
    --cc=sds@tycho.nsa.gov \
    --cc=selinux@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.