History

signalfd simplification

This simplifies signalfd code, by avoiding it to remain attached to the sighand during its lifetime. In this way, the signalfd remain attached to the sighand only during poll(2) (and select and epoll) and read(2). This also allows to remove all the custom "tsk == current" checks in kernel/signal.c, since dequeue_signal() will only be called by "current". I think this is also what Ben was suggesting time ago. The external effect of this, is that a thread can extract only its own private signals and the group ones. I think this is an acceptable behaviour, in that those are the signals the thread would be able to fetch w/out signalfd.

Davide Libenzi committed 19 years ago

Commit b8fceee1 parent 9db619e8 files · +39 −217Browse files

Unified · Split

8 files
  1. fs/exec.c +0 −3
  2. fs/signalfd.c +29 −161
  3. include/linux/init_task.h +1 −1
  4. include/linux/sched.h +1 −1
  5. include/linux/signalfd.h +4 −36
  6. kernel/exit.c +0 −9
  7. kernel/fork.c +1 −1
  8. kernel/signal.c +3 −5
fs/exec.c+0−3
@@ -50,7 +50,6 @@ · 49 unmodified lines
5050 #include <linux/tsacct_kern.h>
5151 #include <linux/cn_proc.h>
5252 #include <linux/audit.h>
53−#include <linux/signalfd.h>
5453
5554 #include <asm/uaccess.h>
5655 #include <asm/mmu_context.h>
@@ -784,7 +783,6 @@ static int de_thread(struct task_struct *tsk) · 727 unmodified lines
784783 * and we can just re-use it all.
785784 */
786785 if (atomic_read(&oldsighand->count) <= 1) {
787− signalfd_detach(tsk);
788786 exit_itimers(sig);
789787 return 0;
790788 }
@@ -923,7 +921,6 @@ static int de_thread(struct task_struct *tsk) · 132 unmodified lines
923921 sig->flags = 0;
924922
925923 no_thread_group:
926− signalfd_detach(tsk);
927924 exit_itimers(sig);
928925 if (leader)
929926 release_task(leader);
fs/signalfd.c+29−161
@@ -11,8 +11,10 @@ · 10 unmodified lines
1111 * Now using anonymous inode source.
1212 * Thanks to Oleg Nesterov for useful code review and suggestions.
1313 * More comments and suggestions from Arnd Bergmann.
14− * Sat May 19, 2007: Davi E. M. Arnaut <davi@haxent.com.br>
14+ * Sat May 19, 2007: Davi E. M. Arnaut <davi@haxent.com.br>
1515 * Retrieve multiple signals with one read() call
16+ * Sun Jul 15, 2007: Davide Libenzi <davidel@xmailserver.org>
17+ * Attach to the sighand only during read() and poll().
1618 */
1719
1820 #include <linux/file.h>
@@ -27,102 +29,12 @@ · 8 unmodified lines
2729 #include <linux/signalfd.h>
2830
2931 struct signalfd_ctx {
30− struct list_head lnk;
31− wait_queue_head_t wqh;
3232 sigset_t sigmask;
33− struct task_struct *tsk;
3433 };
3534
36−struct signalfd_lockctx {
37− struct task_struct *tsk;
38− unsigned long flags;
39−};
40−
41−/*
42− * Tries to acquire the sighand lock. We do not increment the sighand
43− * use count, and we do not even pin the task struct, so we need to
44− * do it inside an RCU read lock, and we must be prepared for the
45− * ctx->tsk going to NULL (in signalfd_deliver()), and for the sighand
46− * being detached. We return 0 if the sighand has been detached, or
47− * 1 if we were able to pin the sighand lock.
48− */
49−static int signalfd_lock(struct signalfd_ctx *ctx, struct signalfd_lockctx *lk)
50−{
51− struct sighand_struct *sighand = NULL;
52−
53− rcu_read_lock();
54− lk->tsk = rcu_dereference(ctx->tsk);
55− if (likely(lk->tsk != NULL))
56− sighand = lock_task_sighand(lk->tsk, &lk->flags);
57− rcu_read_unlock();
58−
59− if (!sighand)
60− return 0;
61−
62− if (!ctx->tsk) {
63− unlock_task_sighand(lk->tsk, &lk->flags);
64− return 0;
65− }
66−
67− if (lk->tsk->tgid == current->tgid)
68− lk->tsk = current;
69−
70− return 1;
71−}
72−
73−static void signalfd_unlock(struct signalfd_lockctx *lk)
74−{
75− unlock_task_sighand(lk->tsk, &lk->flags);
76−}
77−
78−/*
79− * This must be called with the sighand lock held.
80− */
81−void signalfd_deliver(struct task_struct *tsk, int sig)
82−{
83− struct sighand_struct *sighand = tsk->sighand;
84− struct signalfd_ctx *ctx, *tmp;
85−
86− BUG_ON(!sig);
87− list_for_each_entry_safe(ctx, tmp, &sighand->signalfd_list, lnk) {
88− /*
89− * We use a negative signal value as a way to broadcast that the
90− * sighand has been orphaned, so that we can notify all the
91− * listeners about this. Remember the ctx->sigmask is inverted,
92− * so if the user is interested in a signal, that corresponding
93− * bit will be zero.
94− */
95− if (sig < 0) {
96− if (ctx->tsk == tsk) {
97− ctx->tsk = NULL;
98− list_del_init(&ctx->lnk);
99− wake_up(&ctx->wqh);
100− }
101− } else {
102− if (!sigismember(&ctx->sigmask, sig))
103− wake_up(&ctx->wqh);
104− }
105− }
106−}
107−
108−static void signalfd_cleanup(struct signalfd_ctx *ctx)
109−{
110− struct signalfd_lockctx lk;
111−
112− /*
113− * This is tricky. If the sighand is gone, we do not need to remove
114− * context from the list, the list itself won't be there anymore.
115− */
116− if (signalfd_lock(ctx, &lk)) {
117− list_del(&ctx->lnk);
118− signalfd_unlock(&lk);
119− }
120− kfree(ctx);
121−}
122−
12335 static int signalfd_release(struct inode *inode, struct file *file)
12436 {
125− signalfd_cleanup(file->private_data);
37+ kfree(file->private_data);
12638 return 0;
12739 }
12840
@@ -130,23 +42,15 @@ static unsigned int signalfd_poll(struct file *file, poll_table *wait) · 1 unmodified line
13042 {
13143 struct signalfd_ctx *ctx = file->private_data;
13244 unsigned int events = 0;
133− struct signalfd_lockctx lk;
13445
135− poll_wait(file, &ctx->wqh, wait);
46+ poll_wait(file, &current->sighand->signalfd_wqh, wait);
13647
137− /*
138− * Let the caller get a POLLIN in this case, ala socket recv() when
139− * the peer disconnects.
140− */
141− if (signalfd_lock(ctx, &lk)) {
142− if ((lk.tsk == current &&
143− next_signal(&lk.tsk->pending, &ctx->sigmask) > 0) ||
144− next_signal(&lk.tsk->signal->shared_pending,
145− &ctx->sigmask) > 0)
146− events |= POLLIN;
147− signalfd_unlock(&lk);
148− } else
48+ spin_lock_irq(&current->sighand->siglock);
49+ if (next_signal(&current->pending, &ctx->sigmask) ||
50+ next_signal(&current->signal->shared_pending,
51+ &ctx->sigmask))
14952 events |= POLLIN;
53+ spin_unlock_irq(&current->sighand->siglock);
15054
15155 return events;
15256 }
@@ -219,59 +123,46 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, siginfo_t *info, · 66 unmodified lines
219123 int nonblock)
220124 {
221125 ssize_t ret;
222− struct signalfd_lockctx lk;
223126 DECLARE_WAITQUEUE(wait, current);
224127
225− if (!signalfd_lock(ctx, &lk))
226− return 0;
227−
228− ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
128+ spin_lock_irq(&current->sighand->siglock);
129+ ret = dequeue_signal(current, &ctx->sigmask, info);
229130 switch (ret) {
230131 case 0:
231132 if (!nonblock)
232133 break;
233134 ret = -EAGAIN;
234135 default:
235− signalfd_unlock(&lk);
136+ spin_unlock_irq(&current->sighand->siglock);
236137 return ret;
237138 }
238139
239− add_wait_queue(&ctx->wqh, &wait);
140+ add_wait_queue(&current->sighand->signalfd_wqh, &wait);
240141 for (;;) {
241142 set_current_state(TASK_INTERRUPTIBLE);
242− ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
243− signalfd_unlock(&lk);
143+ ret = dequeue_signal(current, &ctx->sigmask, info);
244144 if (ret != 0)
245145 break;
246146 if (signal_pending(current)) {
247147 ret = -ERESTARTSYS;
248148 break;
249149 }
150+ spin_unlock_irq(&current->sighand->siglock);
250151 schedule();
251− ret = signalfd_lock(ctx, &lk);
252− if (unlikely(!ret)) {
253− /*
254− * Let the caller read zero byte, ala socket
255− * recv() when the peer disconnect. This test
256− * must be done before doing a dequeue_signal(),
257− * because if the sighand has been orphaned,
258− * the dequeue_signal() call is going to crash
259− * because ->sighand will be long gone.
260− */
261− break;
262− }
152+ spin_lock_irq(&current->sighand->siglock);
263153 }
154+ spin_unlock_irq(&current->sighand->siglock);
264155
265− remove_wait_queue(&ctx->wqh, &wait);
156+ remove_wait_queue(&current->sighand->signalfd_wqh, &wait);
266157 __set_current_state(TASK_RUNNING);
267158
268159 return ret;
269160 }
270161
271162 /*
272− * Returns either the size of a "struct signalfd_siginfo", or zero if the
273− * sighand we are attached to, has been orphaned. The "count" parameter
274− * must be at least the size of a "struct signalfd_siginfo".
163+ * Returns a multiple of the size of a "struct signalfd_siginfo", or a negative
164+ * error code. The "count" parameter must be at least the size of a
165+ * "struct signalfd_siginfo".
275166 */
276167 static ssize_t signalfd_read(struct file *file, char __user *buf, size_t count,
277168 loff_t *ppos)
@@ -287,7 +178,6 @@ static ssize_t signalfd_read(struct file *file, char __user *buf, size_t count, · 9 unmodified lines
287178 return -EINVAL;
288179
289180 siginfo = (struct signalfd_siginfo __user *) buf;
290−
291181 do {
292182 ret = signalfd_dequeue(ctx, &info, nonblock);
293183 if (unlikely(ret <= 0))
@@ -300,7 +190,7 @@ static ssize_t signalfd_read(struct file *file, char __user *buf, size_t count, · 6 unmodified lines
300190 nonblock = 1;
301191 } while (--count);
302192
303− return total ? total : ret;
193+ return total ? total: ret;
304194 }
305195
306196 static const struct file_operations signalfd_fops = {
@@ -309,20 +199,13 @@ static const struct file_operations signalfd_fops = { · 2 unmodified lines
309199 .read = signalfd_read,
310200 };
311201
312−/*
313− * Create a file descriptor that is associated with our signal
314− * state. We can pass it around to others if we want to, but
315− * it will always be _our_ signal state.
316− */
317202 asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemask)
318203 {
319204 int error;
320205 sigset_t sigmask;
321206 struct signalfd_ctx *ctx;
322− struct sighand_struct *sighand;
323207 struct file *file;
324208 struct inode *inode;
325− struct signalfd_lockctx lk;
326209
327210 if (sizemask != sizeof(sigset_t) ||
328211 copy_from_user(&sigmask, user_mask, sizeof(sigmask)))
@@ -335,17 +218,7 @@ asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemas · 6 unmodified lines
335218 if (!ctx)
336219 return -ENOMEM;
337220
338− init_waitqueue_head(&ctx->wqh);
339221 ctx->sigmask = sigmask;
340− ctx->tsk = current->group_leader;
341−
342− sighand = current->sighand;
343− /*
344− * Add this fd to the list of signal listeners.
345− */
346− spin_lock_irq(&sighand->siglock);
347− list_add_tail(&ctx->lnk, &sighand->signalfd_list);
348− spin_unlock_irq(&sighand->siglock);
349222
350223 /*
351224 * When we call this, the initialization must be complete, since
@@ -364,23 +237,18 @@ asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemas · 12 unmodified lines
364237 fput(file);
365238 return -EINVAL;
366239 }
367− /*
368− * We need to be prepared of the fact that the sighand this fd
369− * is attached to, has been detched. In that case signalfd_lock()
370− * will return 0, and we'll just skip setting the new mask.
371− */
372− if (signalfd_lock(ctx, &lk)) {
373− ctx->sigmask = sigmask;
374− signalfd_unlock(&lk);
375− }
376− wake_up(&ctx->wqh);
240+ spin_lock_irq(&current->sighand->siglock);
241+ ctx->sigmask = sigmask;
242+ spin_unlock_irq(&current->sighand->siglock);
243+
244+ wake_up(&current->sighand->signalfd_wqh);
377245 fput(file);
378246 }
379247
380248 return ufd;
381249
382250 err_fdalloc:
383− signalfd_cleanup(ctx);
251+ kfree(ctx);
384252 return error;
385253 }
386254
include/linux/init_task.h+1−1
@@ -86,7 +86,7 @@ extern struct nsproxy init_nsproxy; · 85 unmodified lines
8686 .count = ATOMIC_INIT(1), \
8787 .action = { { { .sa_handler = NULL, } }, }, \
8888 .siglock = __SPIN_LOCK_UNLOCKED(sighand.siglock), \
89− .signalfd_list = LIST_HEAD_INIT(sighand.signalfd_list), \
89+ .signalfd_wqh = __WAIT_QUEUE_HEAD_INITIALIZER(sighand.signalfd_wqh), \
9090 }
9191
9292 extern struct group_info init_groups;
include/linux/sched.h+1−1
@@ -438,7 +438,7 @@ struct sighand_struct { · 437 unmodified lines
438438 atomic_t count;
439439 struct k_sigaction action[_NSIG];
440440 spinlock_t siglock;
441− struct list_head signalfd_list;
441+ wait_queue_head_t signalfd_wqh;
442442 };
443443
444444 struct pacct_struct {
include/linux/signalfd.h+4−36
@@ -45,49 +45,17 @@ struct signalfd_siginfo { · 44 unmodified lines
4545 #ifdef CONFIG_SIGNALFD
4646
4747 /*
48− * Deliver the signal to listening signalfd. This must be called
49− * with the sighand lock held. Same are the following that end up
50− * calling signalfd_deliver().
51− */
52−void signalfd_deliver(struct task_struct *tsk, int sig);
53−
54−/*
55− * No need to fall inside signalfd_deliver() if no signal listeners
56− * are available.
48+ * Deliver the signal to listening signalfd.
5749 */
5850 static inline void signalfd_notify(struct task_struct *tsk, int sig)
5951 {
60− if (unlikely(!list_empty(&tsk->sighand->signalfd_list)))
61− signalfd_deliver(tsk, sig);
62−}
63−
64−/*
65− * The signal -1 is used to notify the signalfd that the sighand
66− * is on its way to be detached.
67− */
68−static inline void signalfd_detach_locked(struct task_struct *tsk)
69−{
70− if (unlikely(!list_empty(&tsk->sighand->signalfd_list)))
71− signalfd_deliver(tsk, -1);
72−}
73−
74−static inline void signalfd_detach(struct task_struct *tsk)
75−{
76− struct sighand_struct *sighand = tsk->sighand;
77−
78− if (unlikely(!list_empty(&sighand->signalfd_list))) {
79− spin_lock_irq(&sighand->siglock);
80− signalfd_deliver(tsk, -1);
81− spin_unlock_irq(&sighand->siglock);
82− }
52+ if (unlikely(waitqueue_active(&tsk->sighand->signalfd_wqh)))
53+ wake_up(&tsk->sighand->signalfd_wqh);
8354 }
8455
8556 #else /* CONFIG_SIGNALFD */
8657
87−#define signalfd_deliver(t, s) do { } while (0)
88−#define signalfd_notify(t, s) do { } while (0)
89−#define signalfd_detach_locked(t) do { } while (0)
90−#define signalfd_detach(t) do { } while (0)
58+static inline void signalfd_notify(struct task_struct *tsk, int sig) { }
9159
9260 #endif /* CONFIG_SIGNALFD */
9361
kernel/exit.c+0−9
@@ -24,7 +24,6 @@ · 23 unmodified lines
2424 #include <linux/pid_namespace.h>
2525 #include <linux/ptrace.h>
2626 #include <linux/profile.h>
27−#include <linux/signalfd.h>
2827 #include <linux/mount.h>
2928 #include <linux/proc_fs.h>
3029 #include <linux/kthread.h>
@@ -86,14 +85,6 @@ static void __exit_signal(struct task_struct *tsk) · 55 unmodified lines
8685 sighand = rcu_dereference(tsk->sighand);
8786 spin_lock(&sighand->siglock);
8887
89− /*
90− * Notify that this sighand has been detached. This must
91− * be called with the tsk->sighand lock held. Also, this
92− * access tsk->sighand internally, so it must be called
93− * before tsk->sighand is reset.
94− */
95− signalfd_detach_locked(tsk);
96−
9788 posix_cpu_timers_exit(tsk);
9889 if (atomic_dec_and_test(&sig->count))
9990 posix_cpu_timers_exit_group(tsk);
kernel/fork.c+1−1
@@ -1438,7 +1438,7 @@ static void sighand_ctor(void *data, struct kmem_cache *cachep, · 1437 unmodified lines
14381438 struct sighand_struct *sighand = data;
14391439
14401440 spin_lock_init(&sighand->siglock);
1441− INIT_LIST_HEAD(&sighand->signalfd_list);
1441+ init_waitqueue_head(&sighand->signalfd_wqh);
14421442 }
14431443
14441444 void __init proc_caches_init(void)
kernel/signal.c+3−5
@@ -378,8 +378,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info) · 377 unmodified lines
378378 /* We only dequeue private signals from ourselves, we don't let
379379 * signalfd steal them
380380 */
381− if (likely(tsk == current))
382− signr = __dequeue_signal(&tsk->pending, mask, info);
381+ signr = __dequeue_signal(&tsk->pending, mask, info);
383382 if (!signr) {
384383 signr = __dequeue_signal(&tsk->signal->shared_pending,
385384 mask, info);
@@ -407,8 +406,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info) · 21 unmodified lines
407406 }
408407 }
409408 }
410− if (likely(tsk == current))
411− recalc_sigpending();
409+ recalc_sigpending();
412410 if (signr && unlikely(sig_kernel_stop(signr))) {
413411 /*
414412 * Set a marker that we have dequeued a stop signal. Our
@@ -425,7 +423,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info) · 10 unmodified lines
425423 if (!(tsk->signal->flags & SIGNAL_GROUP_EXIT))
426424 tsk->signal->flags |= SIGNAL_STOP_DEQUEUED;
427425 }
428− if (signr && likely(tsk == current) &&
426+ if (signr &&
429427 ((info->si_code & __SI_MASK) == __SI_TIMER) &&
430428 info->si_sys_private){
431429 /*

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t