From: Joe Korty <joe.korty@concurrent-rt.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-rt-users@vger.kernel.org,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: [PATCH] signals: Allow tasks to cache one sigqueue struct, version 2
Date: Thu, 30 Apr 2020 12:18:48 -0400 [thread overview]
Message-ID: <20200430161848.GA29113@zipoli.concurrent-rt.com> (raw)
In-Reply-To: <20200428154536.GA33300@zipoli.concurrent-rt.com>
[5.4.28-rt19] Cache up to one sigqueue per task.
This replaces an earlier version of this rt patch,
signals-allow-rt-tasks-to-cache-one-sigqueue-struct.patch
which is able to generate a kernel splat of the form,
virt_to_cache: Object is not a Slab page!
...
Call Trace:
free_uid+0x93/0xa0
__dequeue_signal+0x21c/0x230
dequeue_signal+0xb7/0x1b0
get_signal+0x266/0xce0
do_signal+0x36/0x640
This most easily happens when the test program, tb.c,
is executed. That test program can be found in an
earlier linux-rt-users posting,
Kill(2) and pthread_create(2) interact poorly in 5.4.28-rt19
Date: Tue, 28 Apr 2020 11:45:36 -0400
After poking around the original patch, not making any
progress, I tried revamping the patch down to its simpliest
possible implementation. This revamped version has been
tested in both an rt and nort compiled 5.4.28-rt19 kernel.
No issues have been seen so far.
Please consider
1) dropping the original cache-sigqueue patch, or
2) replace that patch with this new version, or
3) rewrite the original patch using your own criteria
for what a correct patch should be.
and backport whatever fix is selected to all versions of the
rt kernel that have the original patch applied.
Thanks,
Joe
---
signals: Allow tasks to cache one sigqueue struct, version 2
To avoid allocation overhead, allow all tasks to cache one sigqueue struct
in task struct.
[ This replaces version 1, which is unstable in several versions of the
rt patch. ]
Signed-off-by: Joe Korty <joe.korty@concurrent-rt.com>
Index: b/include/linux/sched.h
===================================================================
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -935,6 +935,7 @@ struct task_struct {
/* Signal handlers: */
struct signal_struct *signal;
struct sighand_struct *sighand;
+ struct sigqueue *sigqueue_cache;
sigset_t blocked;
sigset_t real_blocked;
/* Restored if set_restore_sigmask() was used: */
Index: b/include/linux/signal.h
===================================================================
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -255,6 +255,7 @@ static inline void init_sigpending(struc
}
extern void flush_sigqueue(struct sigpending *queue);
+extern void flush_task_sigqueue(struct task_struct *tsk);
/* Test if 'sig' is valid signal. Use this instead of testing _NSIG directly */
static inline int valid_signal(unsigned long sig)
Index: b/kernel/exit.c
===================================================================
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -171,6 +171,7 @@ static void __exit_signal(struct task_st
flush_sigqueue(&sig->shared_pending);
tty_kref_put(tty);
}
+ flush_task_sigqueue(tsk);
}
static void delayed_put_task_struct(struct rcu_head *rhp)
Index: b/kernel/fork.c
===================================================================
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1587,6 +1587,7 @@ static int copy_signal(unsigned long clo
init_waitqueue_head(&sig->wait_chldexit);
sig->curr_target = tsk;
init_sigpending(&sig->shared_pending);
+ tsk->sigqueue_cache = NULL;
INIT_HLIST_HEAD(&sig->multiprocess);
seqlock_init(&sig->stats_lock);
prev_cputime_init(&sig->prev_cputime);
@@ -1924,6 +1925,7 @@ static __latent_entropy struct task_stru
spin_lock_init(&p->alloc_lock);
init_sigpending(&p->pending);
+ p->sigqueue_cache = NULL;
p->utime = p->stime = p->gtime = 0;
#ifdef CONFIG_ARCH_HAS_SCALED_CPUTIME
Index: b/kernel/signal.c
===================================================================
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -431,7 +431,9 @@ __sigqueue_alloc(int sig, struct task_st
rcu_read_unlock();
if (override_rlimit || likely(sigpending <= task_rlimit(t, RLIMIT_SIGPENDING))) {
- q = kmem_cache_alloc(sigqueue_cachep, flags);
+ q = xchg(&t->sigqueue_cache, NULL);
+ if (!q)
+ q = kmem_cache_alloc(sigqueue_cachep, flags);
} else {
print_dropped_signal(sig);
}
@@ -454,7 +456,9 @@ static void __sigqueue_free(struct sigqu
return;
if (atomic_dec_and_test(&q->user->sigpending))
free_uid(q->user);
- kmem_cache_free(sigqueue_cachep, q);
+ q = xchg(¤t->sigqueue_cache, q);
+ if (q)
+ kmem_cache_free(sigqueue_cachep, q);
}
void flush_sigqueue(struct sigpending *queue)
@@ -469,6 +473,15 @@ void flush_sigqueue(struct sigpending *q
}
}
+void flush_task_sigqueue(struct task_struct *t)
+{
+ struct sigqueue *q;
+
+ q = xchg(&t->sigqueue_cache, NULL);
+ if (q)
+ kmem_cache_free(sigqueue_cachep, q);
+}
+
/*
* Flush all pending signals for this kthread.
*/
next prev parent reply other threads:[~2020-04-30 16:19 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 15:45 Kill(2) and pthread_create(2) interact poorly in 5.4.28-rt19 Joe Korty
2020-04-30 16:18 ` Joe Korty [this message]
2020-05-04 15:41 ` [PATCH] signals: Allow tasks to cache one sigqueue struct, version 2 Sebastian Andrzej Siewior
2020-05-04 16:27 ` Joe Korty
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=20200430161848.GA29113@zipoli.concurrent-rt.com \
--to=joe.korty@concurrent-rt.com \
--cc=bigeasy@linutronix.de \
--cc=linux-rt-users@vger.kernel.org \
--cc=tglx@linutronix.de \
/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).