Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[PATCH] PM: Fix SMP races in the freezer

Currently, to tell a task that it should go to the refrigerator, we set the
PF_FREEZE flag for it and send a fake signal to it. Unfortunately there
are two SMP-related problems with this approach. First, a task running on
another CPU may be updating its flags while the freezer attempts to set
PF_FREEZE for it and this may leave the task's flags in an inconsistent
state. Second, there is a potential race between freeze_process() and
refrigerator() in which freeze_process() running on one CPU is reading a
task's PF_FREEZE flag while refrigerator() running on another CPU has just
set PF_FROZEN for the same task and attempts to reset PF_FREEZE for it. If
the refrigerator wins the race, freeze_process() will state that PF_FREEZE
hasn't been set for the task and will set it unnecessarily, so the task
will go to the refrigerator once again after it's been thawed.

To solve first of these problems we need to stop using PF_FREEZE to tell
tasks that they should go to the refrigerator. Instead, we can introduce a
special TIF_*** flag and use it for this purpose, since it is allowed to
change the other tasks' TIF_*** flags and there are special calls for it.

To avoid the freeze_process()-refrigerator() race we can make
freeze_process() to always check the task's PF_FROZEN flag after it's read
its "freeze" flag. We should also make sure that refrigerator() will
always reset the task's "freeze" flag after it's set PF_FROZEN for it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: David Howells <dhowells@redhat.com>
Cc: Andi Kleen <ak@muc.de>
Cc: "Luck, Tony" <tony.luck@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

authored by

Rafael J. Wysocki and committed by
Linus Torvalds
8a102eed 3df494a3

+29 -12
+2
include/asm-arm/thread_info.h
··· 147 147 #define TIF_POLLING_NRFLAG 16 148 148 #define TIF_USING_IWMMXT 17 149 149 #define TIF_MEMDIE 18 150 + #define TIF_FREEZE 19 150 151 151 152 #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) 152 153 #define _TIF_SIGPENDING (1 << TIF_SIGPENDING) ··· 155 154 #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) 156 155 #define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG) 157 156 #define _TIF_USING_IWMMXT (1 << TIF_USING_IWMMXT) 157 + #define _TIF_FREEZE (1 << TIF_FREEZE) 158 158 159 159 /* 160 160 * Change these and you break ASM code in entry-common.S
+2
include/asm-frv/thread_info.h
··· 116 116 #define TIF_RESTORE_SIGMASK 6 /* restore signal mask in do_signal() */ 117 117 #define TIF_POLLING_NRFLAG 16 /* true if poll_idle() is polling TIF_NEED_RESCHED */ 118 118 #define TIF_MEMDIE 17 /* OOM killer killed process */ 119 + #define TIF_FREEZE 18 /* freezing for suspend */ 119 120 120 121 #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) 121 122 #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) ··· 126 125 #define _TIF_IRET (1 << TIF_IRET) 127 126 #define _TIF_RESTORE_SIGMASK (1 << TIF_RESTORE_SIGMASK) 128 127 #define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG) 128 + #define _TIF_FREEZE (1 << TIF_FREEZE) 129 129 130 130 #define _TIF_WORK_MASK 0x0000FFFE /* work to do on interrupt/exception return */ 131 131 #define _TIF_ALLWORK_MASK 0x0000FFFF /* work to do on any return to u-space */
+2
include/asm-i386/thread_info.h
··· 134 134 #define TIF_MEMDIE 16 135 135 #define TIF_DEBUG 17 /* uses debug registers */ 136 136 #define TIF_IO_BITMAP 18 /* uses I/O bitmap */ 137 + #define TIF_FREEZE 19 /* is freezing for suspend */ 137 138 138 139 #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) 139 140 #define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME) ··· 148 147 #define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) 149 148 #define _TIF_DEBUG (1<<TIF_DEBUG) 150 149 #define _TIF_IO_BITMAP (1<<TIF_IO_BITMAP) 150 + #define _TIF_FREEZE (1<<TIF_FREEZE) 151 151 152 152 /* work to do on interrupt/exception return */ 153 153 #define _TIF_WORK_MASK \
+2
include/asm-ia64/thread_info.h
··· 88 88 #define TIF_MEMDIE 17 89 89 #define TIF_MCA_INIT 18 /* this task is processing MCA or INIT */ 90 90 #define TIF_DB_DISABLED 19 /* debug trap disabled for fsyscall */ 91 + #define TIF_FREEZE 20 /* is freezing for suspend */ 91 92 92 93 #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) 93 94 #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) ··· 99 98 #define _TIF_POLLING_NRFLAG (1 << TIF_POLLING_NRFLAG) 100 99 #define _TIF_MCA_INIT (1 << TIF_MCA_INIT) 101 100 #define _TIF_DB_DISABLED (1 << TIF_DB_DISABLED) 101 + #define _TIF_FREEZE (1 << TIF_FREEZE) 102 102 103 103 /* "work to do on user-return" bits */ 104 104 #define TIF_ALLWORK_MASK (_TIF_NOTIFY_RESUME|_TIF_SIGPENDING|_TIF_NEED_RESCHED|_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT)
+2
include/asm-powerpc/thread_info.h
··· 122 122 #define TIF_RESTOREALL 12 /* Restore all regs (implies NOERROR) */ 123 123 #define TIF_NOERROR 14 /* Force successful syscall return */ 124 124 #define TIF_RESTORE_SIGMASK 15 /* Restore signal mask in do_signal */ 125 + #define TIF_FREEZE 16 /* Freezing for suspend */ 125 126 126 127 /* as above, but as bit values */ 127 128 #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) ··· 139 138 #define _TIF_RESTOREALL (1<<TIF_RESTOREALL) 140 139 #define _TIF_NOERROR (1<<TIF_NOERROR) 141 140 #define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) 141 + #define _TIF_FREEZE (1<<TIF_FREEZE) 142 142 #define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP) 143 143 144 144 #define _TIF_USER_WORK_MASK (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \
+2
include/asm-sh/thread_info.h
··· 106 106 #define TIF_USEDFPU 16 /* FPU was used by this task this quantum (SMP) */ 107 107 #define TIF_POLLING_NRFLAG 17 /* true if poll_idle() is polling TIF_NEED_RESCHED */ 108 108 #define TIF_MEMDIE 18 109 + #define TIF_FREEZE 19 109 110 110 111 #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) 111 112 #define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME) ··· 115 114 #define _TIF_RESTORE_SIGMASK (1<<TIF_RESTORE_SIGMASK) 116 115 #define _TIF_USEDFPU (1<<TIF_USEDFPU) 117 116 #define _TIF_POLLING_NRFLAG (1<<TIF_POLLING_NRFLAG) 117 + #define _TIF_FREEZE (1<<TIF_FREEZE) 118 118 119 119 #define _TIF_WORK_MASK 0x000000FE /* work to do on interrupt/exception return */ 120 120 #define _TIF_ALLWORK_MASK 0x000000FF /* work to do on any return to u-space */
+2
include/asm-x86_64/thread_info.h
··· 122 122 #define TIF_MEMDIE 20 123 123 #define TIF_DEBUG 21 /* uses debug registers */ 124 124 #define TIF_IO_BITMAP 22 /* uses I/O bitmap */ 125 + #define TIF_FREEZE 23 /* is freezing for suspend */ 125 126 126 127 #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE) 127 128 #define _TIF_NOTIFY_RESUME (1<<TIF_NOTIFY_RESUME) ··· 138 137 #define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING) 139 138 #define _TIF_DEBUG (1<<TIF_DEBUG) 140 139 #define _TIF_IO_BITMAP (1<<TIF_IO_BITMAP) 140 + #define _TIF_FREEZE (1<<TIF_FREEZE) 141 141 142 142 /* work to do on interrupt/exception return */ 143 143 #define _TIF_WORK_MASK \
+6 -5
include/linux/freezer.h
··· 16 16 */ 17 17 static inline int freezing(struct task_struct *p) 18 18 { 19 - return p->flags & PF_FREEZE; 19 + return test_tsk_thread_flag(p, TIF_FREEZE); 20 20 } 21 21 22 22 /* 23 23 * Request that a process be frozen 24 - * FIXME: SMP problem. We may not modify other process' flags! 25 24 */ 26 25 static inline void freeze(struct task_struct *p) 27 26 { 28 - p->flags |= PF_FREEZE; 27 + set_tsk_thread_flag(p, TIF_FREEZE); 29 28 } 30 29 31 30 /* ··· 32 33 */ 33 34 static inline void do_not_freeze(struct task_struct *p) 34 35 { 35 - p->flags &= ~PF_FREEZE; 36 + clear_tsk_thread_flag(p, TIF_FREEZE); 36 37 } 37 38 38 39 /* ··· 53 54 */ 54 55 static inline void frozen_process(struct task_struct *p) 55 56 { 56 - p->flags = (p->flags & ~PF_FREEZE) | PF_FROZEN; 57 + p->flags |= PF_FROZEN; 58 + wmb(); 59 + clear_tsk_thread_flag(p, TIF_FREEZE); 57 60 } 58 61 59 62 extern void refrigerator(void);
-1
include/linux/sched.h
··· 1144 1144 #define PF_MEMALLOC 0x00000800 /* Allocating memory */ 1145 1145 #define PF_FLUSHER 0x00001000 /* responsible for disk writeback */ 1146 1146 #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ 1147 - #define PF_FREEZE 0x00004000 /* this task is being frozen for suspend now */ 1148 1147 #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ 1149 1148 #define PF_FROZEN 0x00010000 /* frozen for system suspend */ 1150 1149 #define PF_FSTRANS 0x00020000 /* inside a filesystem transaction */
+9 -6
kernel/power/process.c
··· 60 60 unsigned long flags; 61 61 62 62 if (!freezing(p)) { 63 - if (p->state == TASK_STOPPED) 64 - force_sig_specific(SIGSTOP, p); 63 + rmb(); 64 + if (!frozen(p)) { 65 + if (p->state == TASK_STOPPED) 66 + force_sig_specific(SIGSTOP, p); 65 67 66 - freeze(p); 67 - spin_lock_irqsave(&p->sighand->siglock, flags); 68 - signal_wake_up(p, p->state == TASK_STOPPED); 69 - spin_unlock_irqrestore(&p->sighand->siglock, flags); 68 + freeze(p); 69 + spin_lock_irqsave(&p->sighand->siglock, flags); 70 + signal_wake_up(p, p->state == TASK_STOPPED); 71 + spin_unlock_irqrestore(&p->sighand->siglock, flags); 72 + } 70 73 } 71 74 } 72 75