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 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.