From mboxrd@z Thu Jan 1 00:00:00 1970 Date: Sun, 26 Jun 2005 22:53:56 -0700 (PDT) From: Christoph Lameter Subject: Re: [RFC] Fix SMP brokenness for PF_FREEZE and make freezing usable for other purposes In-Reply-To: Message-ID: References: <20050625025122.GC22393@atrey.karlin.mff.cuni.cz> <20050626023053.GA2871@atrey.karlin.mff.cuni.cz> <20050626030925.GA4156@atrey.karlin.mff.cuni.cz> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-linux-mm@kvack.org Return-Path: To: Linus Torvalds Cc: Pavel Machek , linux-mm@kvack.org, linux-kernel@vger.kernel.org, raybry@engr.sgi.com List-ID: On Sun, 26 Jun 2005, Linus Torvalds wrote: > It's called "work", and we have the "TIF_xxx" flags for it. That's how > "need-resched" and "sigpending" are done. There could be a > "TIF_FREEZEPENDING" thing there too.. Ok. Here is yet another version of the patch: --- The current suspend code modifies thread flags from outside the context of process. This creates a SMP race. The patch fixes that by introducing a TIF_FREEZE flag (for all arches). Also - Uses a completion handler instead of waiting in a schedule loop in the refrigerator. - Introduces a semaphore freezer_sem to provide a way that multiple kernel subsystems can use the freezing ability without interfering with one another. - Include necessary definitions for the migration code if CONFIG_MIGRATE is set. - Removes PF_FREEZE If this approach is okay then we will need to move the refrigerator() and the definition of the semaphore and the completion variable out of kernel/power/process.c into kernel/sched.c (right?). Signed-off-by: Christoph Lameter Index: linux-2.6.12/include/linux/sched.h =================================================================== --- linux-2.6.12.orig/include/linux/sched.h 2005-06-27 05:20:15.000000000 +0000 +++ linux-2.6.12/include/linux/sched.h 2005-06-27 05:33:46.000000000 +0000 @@ -804,7 +804,6 @@ do { if (atomic_dec_and_test(&(tsk)->usa #define PF_MEMALLOC 0x00000800 /* Allocating memory */ #define PF_FLUSHER 0x00001000 /* responsible for disk writeback */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ -#define PF_FREEZE 0x00004000 /* this task is being frozen for suspend now */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ #define PF_FROZEN 0x00010000 /* frozen for system suspend */ #define PF_FSTRANS 0x00020000 /* inside a filesystem transaction */ @@ -1265,78 +1264,37 @@ extern void normalize_rt_tasks(void); #endif -#ifdef CONFIG_PM -/* - * Check if a process has been frozen - */ -static inline int frozen(struct task_struct *p) -{ - return p->flags & PF_FROZEN; -} - -/* - * Check if there is a request to freeze a process - */ -static inline int freezing(struct task_struct *p) -{ - return p->flags & PF_FREEZE; -} +#if defined(CONFIG_PM) || defined(CONFIG_MIGRATE) +extern struct semaphore freezer_sem; +extern struct completion thaw; -/* - * Request that a process be frozen - * FIXME: SMP problem. We may not modify other process' flags! - */ -static inline void freeze(struct task_struct *p) -{ - p->flags |= PF_FREEZE; -} - -/* - * Wake up a frozen process - */ -static inline int thaw_process(struct task_struct *p) -{ - if (frozen(p)) { - p->flags &= ~PF_FROZEN; - wake_up_process(p); - return 1; - } - return 0; -} +extern void refrigerator(void); -/* - * freezing is complete, mark process as frozen - */ -static inline void frozen_process(struct task_struct *p) +static inline int freezing(struct task_struct *p) { - p->flags = (p->flags & ~PF_FREEZE) | PF_FROZEN; + return test_ti_thread_flag(p->thread_info, TIF_FREEZE); } -extern void refrigerator(void); -extern int freeze_processes(void); -extern void thaw_processes(void); - static inline int try_to_freeze(void) { - if (freezing(current)) { + if (test_thread_flag(TIF_FREEZE)) { refrigerator(); return 1; } else return 0; } #else -static inline int frozen(struct task_struct *p) { return 0; } +static inline void refrigerator(void) {} static inline int freezing(struct task_struct *p) { return 0; } -static inline void freeze(struct task_struct *p) { BUG(); } -static inline int thaw_process(struct task_struct *p) { return 1; } -static inline void frozen_process(struct task_struct *p) { BUG(); } +static inline int try_to_freeze(void) { return 0; } +#endif -static inline void refrigerator(void) {} +#ifdef CONFIG_PM +extern int freeze_processes(void); +extern void thaw_processes(void); +#else static inline int freeze_processes(void) { BUG(); return 0; } static inline void thaw_processes(void) {} - -static inline int try_to_freeze(void) { return 0; } - #endif /* CONFIG_PM */ #endif /* __KERNEL__ */ Index: linux-2.6.12/kernel/power/process.c =================================================================== --- linux-2.6.12.orig/kernel/power/process.c 2005-06-27 05:20:15.000000000 +0000 +++ linux-2.6.12/kernel/power/process.c 2005-06-27 05:22:00.000000000 +0000 @@ -18,6 +18,8 @@ */ #define TIMEOUT (6 * HZ) +DECLARE_MUTEX(freezer_sem); +DECLARE_COMPLETION(thaw); static inline int freezeable(struct task_struct * p) { @@ -31,27 +33,18 @@ static inline int freezeable(struct task return 1; } -/* Refrigerator is place where frozen processes are stored :-). */ void refrigerator(void) { - /* Hmm, should we be allowed to suspend when there are realtime - processes around? */ - long save; - save = current->state; - current->state = TASK_UNINTERRUPTIBLE; - pr_debug("%s entered refrigerator\n", current->comm); - printk("="); - - frozen_process(current); + current->flags |= PF_FROZEN; + clear_thread_flag(TIF_FREEZE); + /* A fake signal 0 may have been sent. Recalculate sigpending */ spin_lock_irq(¤t->sighand->siglock); - recalc_sigpending(); /* We sent fake signal, clean it up */ + recalc_sigpending(); spin_unlock_irq(¤t->sighand->siglock); - - while (frozen(current)) - schedule(); - pr_debug("%s left refrigerator\n", current->comm); - current->state = save; + wait_for_completion(&thaw); + current->flags &= ~PF_FROZEN; } +EXPORT_SYMBOL(refrigerator); /* 0 = success, else # of processes that we failed to stop */ int freeze_processes(void) @@ -60,6 +53,7 @@ int freeze_processes(void) unsigned long start_time; struct task_struct *g, *p; + down(&freezer_sem); printk( "Stopping tasks: " ); start_time = jiffies; do { @@ -69,12 +63,12 @@ int freeze_processes(void) unsigned long flags; if (!freezeable(p)) continue; - if ((frozen(p)) || + if ((p->flags & PF_FROZEN) || (p->state == TASK_TRACED) || (p->state == TASK_STOPPED)) continue; - freeze(p); + set_thread_flag(TIF_FREEZE); spin_lock_irqsave(&p->sighand->siglock, flags); signal_wake_up(p, 0); spin_unlock_irqrestore(&p->sighand->siglock, flags); @@ -96,20 +90,6 @@ int freeze_processes(void) void thaw_processes(void) { - struct task_struct *g, *p; - - printk( "Restarting tasks..." ); - read_lock(&tasklist_lock); - do_each_thread(g, p) { - if (!freezeable(p)) - continue; - if (!thaw_process(p)) - printk(KERN_INFO " Strange, %s not stopped\n", p->comm ); - } while_each_thread(g, p); - - read_unlock(&tasklist_lock); - schedule(); - printk( " done\n" ); + complete_all(&thaw); + up(&freezer_sem); } - -EXPORT_SYMBOL(refrigerator); Index: linux-2.6.12/include/asm-x86_64/thread_info.h =================================================================== --- linux-2.6.12.orig/include/asm-x86_64/thread_info.h 2005-06-27 05:20:15.000000000 +0000 +++ linux-2.6.12/include/asm-x86_64/thread_info.h 2005-06-27 05:37:24.000000000 +0000 @@ -108,6 +108,7 @@ static inline struct thread_info *stack_ #define TIF_FORK 18 /* ret_from_fork */ #define TIF_ABI_PENDING 19 #define TIF_MEMDIE 20 +#define TIF_FREEZE 21 /* Freeze process */ #define _TIF_SYSCALL_TRACE (1< aart@kvack.org