diff options
author | Oleg Nesterov <oleg@redhat.com> | 2010-05-26 17:43:16 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2010-05-27 12:12:46 -0400 |
commit | ea6d290ca34c4fd91b7348338c0cc7bdeff94a35 (patch) | |
tree | 6e9bd367650d9233c5b6cf1059845f17cb1bc460 | |
parent | 4dec2a91fd7e8815d730afbfdcf085cbf53433ac (diff) |
signals: make task_struct->signal immutable/refcountable
We have a lot of problems with accessing task_struct->signal, it can
"disappear" at any moment. Even current can't use its ->signal safely
after exit_notify(). ->siglock helps, but it is not convenient, not
always possible, and sometimes it makes sense to use task->signal even
after this task has already dead.
This patch adds the reference counter, sigcnt, into signal_struct. This
reference is owned by task_struct and it is dropped in
__put_task_struct(). Perhaps it makes sense to export
get/put_signal_struct() later, but currently I don't see the immediate
reason.
Rename __cleanup_signal() to free_signal_struct() and unexport it. With
the previous changes it does nothing except kmem_cache_free().
Change __exit_signal() to not clear/free ->signal, it will be freed when
the last reference to any thread in the thread group goes away.
Note:
- when the last thead exits signal->tty can point to nowhere, see
the next patch.
- with or without this patch signal_struct->count should go away,
or at least it should be "int nr_threads" for fs/proc. This will
be addressed later.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Acked-by: Roland McGrath <roland@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | include/linux/sched.h | 2 | ||||
-rw-r--r-- | kernel/exit.c | 3 | ||||
-rw-r--r-- | kernel/fork.c | 23 |
3 files changed, 17 insertions, 11 deletions
diff --git a/include/linux/sched.h b/include/linux/sched.h index a95a2455cebe..32e309df408c 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h | |||
@@ -527,6 +527,7 @@ struct thread_group_cputimer { | |||
527 | * the locking of signal_struct. | 527 | * the locking of signal_struct. |
528 | */ | 528 | */ |
529 | struct signal_struct { | 529 | struct signal_struct { |
530 | atomic_t sigcnt; | ||
530 | atomic_t count; | 531 | atomic_t count; |
531 | atomic_t live; | 532 | atomic_t live; |
532 | 533 | ||
@@ -2101,7 +2102,6 @@ extern void flush_thread(void); | |||
2101 | extern void exit_thread(void); | 2102 | extern void exit_thread(void); |
2102 | 2103 | ||
2103 | extern void exit_files(struct task_struct *); | 2104 | extern void exit_files(struct task_struct *); |
2104 | extern void __cleanup_signal(struct signal_struct *); | ||
2105 | extern void __cleanup_sighand(struct sighand_struct *); | 2105 | extern void __cleanup_sighand(struct sighand_struct *); |
2106 | 2106 | ||
2107 | extern void exit_itimers(struct signal_struct *); | 2107 | extern void exit_itimers(struct signal_struct *); |
diff --git a/kernel/exit.c b/kernel/exit.c index 4a72f1753edb..92af5cde9bbe 100644 --- a/kernel/exit.c +++ b/kernel/exit.c | |||
@@ -134,8 +134,6 @@ static void __exit_signal(struct task_struct *tsk) | |||
134 | * doing sigqueue_free() if we have SIGQUEUE_PREALLOC signals. | 134 | * doing sigqueue_free() if we have SIGQUEUE_PREALLOC signals. |
135 | */ | 135 | */ |
136 | flush_sigqueue(&tsk->pending); | 136 | flush_sigqueue(&tsk->pending); |
137 | |||
138 | tsk->signal = NULL; | ||
139 | tsk->sighand = NULL; | 137 | tsk->sighand = NULL; |
140 | spin_unlock(&sighand->siglock); | 138 | spin_unlock(&sighand->siglock); |
141 | 139 | ||
@@ -150,7 +148,6 @@ static void __exit_signal(struct task_struct *tsk) | |||
150 | */ | 148 | */ |
151 | task_rq_unlock_wait(tsk); | 149 | task_rq_unlock_wait(tsk); |
152 | tty_kref_put(sig->tty); | 150 | tty_kref_put(sig->tty); |
153 | __cleanup_signal(sig); | ||
154 | } | 151 | } |
155 | } | 152 | } |
156 | 153 | ||
diff --git a/kernel/fork.c b/kernel/fork.c index b7879ef6e7cd..e08e3012cd6b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c | |||
@@ -165,6 +165,18 @@ void free_task(struct task_struct *tsk) | |||
165 | } | 165 | } |
166 | EXPORT_SYMBOL(free_task); | 166 | EXPORT_SYMBOL(free_task); |
167 | 167 | ||
168 | static inline void free_signal_struct(struct signal_struct *sig) | ||
169 | { | ||
170 | thread_group_cputime_free(sig); | ||
171 | kmem_cache_free(signal_cachep, sig); | ||
172 | } | ||
173 | |||
174 | static inline void put_signal_struct(struct signal_struct *sig) | ||
175 | { | ||
176 | if (atomic_dec_and_test(&sig->sigcnt)) | ||
177 | free_signal_struct(sig); | ||
178 | } | ||
179 | |||
168 | void __put_task_struct(struct task_struct *tsk) | 180 | void __put_task_struct(struct task_struct *tsk) |
169 | { | 181 | { |
170 | WARN_ON(!tsk->exit_state); | 182 | WARN_ON(!tsk->exit_state); |
@@ -173,6 +185,7 @@ void __put_task_struct(struct task_struct *tsk) | |||
173 | 185 | ||
174 | exit_creds(tsk); | 186 | exit_creds(tsk); |
175 | delayacct_tsk_free(tsk); | 187 | delayacct_tsk_free(tsk); |
188 | put_signal_struct(tsk->signal); | ||
176 | 189 | ||
177 | if (!profile_handoff_task(tsk)) | 190 | if (!profile_handoff_task(tsk)) |
178 | free_task(tsk); | 191 | free_task(tsk); |
@@ -864,6 +877,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) | |||
864 | if (!sig) | 877 | if (!sig) |
865 | return -ENOMEM; | 878 | return -ENOMEM; |
866 | 879 | ||
880 | atomic_set(&sig->sigcnt, 1); | ||
867 | atomic_set(&sig->count, 1); | 881 | atomic_set(&sig->count, 1); |
868 | atomic_set(&sig->live, 1); | 882 | atomic_set(&sig->live, 1); |
869 | init_waitqueue_head(&sig->wait_chldexit); | 883 | init_waitqueue_head(&sig->wait_chldexit); |
@@ -889,12 +903,6 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) | |||
889 | return 0; | 903 | return 0; |
890 | } | 904 | } |
891 | 905 | ||
892 | void __cleanup_signal(struct signal_struct *sig) | ||
893 | { | ||
894 | thread_group_cputime_free(sig); | ||
895 | kmem_cache_free(signal_cachep, sig); | ||
896 | } | ||
897 | |||
898 | static void copy_flags(unsigned long clone_flags, struct task_struct *p) | 906 | static void copy_flags(unsigned long clone_flags, struct task_struct *p) |
899 | { | 907 | { |
900 | unsigned long new_flags = p->flags; | 908 | unsigned long new_flags = p->flags; |
@@ -1248,6 +1256,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, | |||
1248 | } | 1256 | } |
1249 | 1257 | ||
1250 | if (clone_flags & CLONE_THREAD) { | 1258 | if (clone_flags & CLONE_THREAD) { |
1259 | atomic_inc(¤t->signal->sigcnt); | ||
1251 | atomic_inc(¤t->signal->count); | 1260 | atomic_inc(¤t->signal->count); |
1252 | atomic_inc(¤t->signal->live); | 1261 | atomic_inc(¤t->signal->live); |
1253 | p->group_leader = current->group_leader; | 1262 | p->group_leader = current->group_leader; |
@@ -1294,7 +1303,7 @@ bad_fork_cleanup_mm: | |||
1294 | mmput(p->mm); | 1303 | mmput(p->mm); |
1295 | bad_fork_cleanup_signal: | 1304 | bad_fork_cleanup_signal: |
1296 | if (!(clone_flags & CLONE_THREAD)) | 1305 | if (!(clone_flags & CLONE_THREAD)) |
1297 | __cleanup_signal(p->signal); | 1306 | free_signal_struct(p->signal); |
1298 | bad_fork_cleanup_sighand: | 1307 | bad_fork_cleanup_sighand: |
1299 | __cleanup_sighand(p->sighand); | 1308 | __cleanup_sighand(p->sighand); |
1300 | bad_fork_cleanup_fs: | 1309 | bad_fork_cleanup_fs: |