Messages in this thread Patch in this message | | | Date | Sat, 25 Jun 2005 11:35:34 +0400 | From | Kirill Korotaev <> | Subject | Re: [RFC] Fix SMP brokenness for PF_FREEZE and make freezing usable for other purposes |
| |
CL> The process freezing used by software suspend currently relies on modifying current->>flags from outside of the processes context. This makes freezing and CL> unfreezing SMP unsafe since a process may change the flags at any time without CL> locking. The following patch introduces a new atomic_t field in task_struct CL> to allow SMP safe freezing and unfreezing.
CL> It provides a simple API for process freezing:
CL> frozen(process) Check for frozen process CL> freezing(process) Check if a process is being frozen CL> freeze(process) Tell a process to freeze (go to refrigerator) CL> thaw_process(process) Restart process
CL> I only know that this boots correctly since I have no system that can do CL> suspend. But Ray needs an effective means of process suspension for CL> his process migration patches. Just out of curiosity what patches are you talking about? Is it possible to take a glance at it if it is what I think of?
comments below...
[skip] CL> /* Refrigerator is place where frozen processes are stored :-). */ CL> -void refrigerator(unsigned long flag) CL> +void refrigerator(void) CL> { CL> /* Hmm, should we be allowed to suspend when there are realtime CL> processes around? */ CL> @@ -40,15 +40,13 @@ void refrigerator(unsigned long flag) CL> save = current->state; CL> current->state = TASK_UNINTERRUPTIBLE; CL> pr_debug("%s entered refrigerator\n", current->comm); CL> - printk("="); - current->>flags &= ~PF_FREEZE; CL> spin_lock_irq(¤t->sighand->siglock); CL> recalc_sigpending(); /* We sent fake signal, clean it up */ CL> spin_unlock_irq(¤t->sighand->siglock); - current->>flags |= PF_FROZEN; - while (current->>flags & PF_FROZEN) CL> + atomic_set(¤t->freeze_status, FROZEN); <<<< this looks unsafe... what if thaw_process() was called exactly before this atomic_set()? then you won't wake up anymore... CL> + while (frozen(current)) CL> schedule(); CL> pr_debug("%s left refrigerator\n", current->comm); CL> current->state = save; CL> @@ -57,10 +55,10 @@ void refrigerator(unsigned long flag) CL> /* 0 = success, else # of processes that we failed to stop */ CL> int freeze_processes(void) CL> { CL> - int todo; CL> - unsigned long start_time; CL> + int todo; CL> + unsigned long start_time; CL> struct task_struct *g, *p; CL> - CL> + CL> printk( "Stopping tasks: " ); CL> start_time = jiffies; CL> do { CL> @@ -70,14 +68,12 @@ int freeze_processes(void) CL> unsigned long flags; CL> if (!freezeable(p)) CL> continue; - if ((p->>flags & PF_FROZEN) || CL> + if ((frozen(p)) || <<<< this looks unsafe also... i think you have to check freezing() instead of frozen(). otherwise you will be constantly incrementing freezing counter and it can potentially overflow... Though this is unlikely I see no reason to call freeze() if we are freezing already... But we can call signal_wake_up() to speed up the process... CL> (p->state == TASK_TRACED) || CL> (p->state == TASK_STOPPED)) CL> continue; CL> - /* FIXME: smp problem here: we may not access other process' flags CL> - without locking */ - p->>flags |= PF_FREEZE; CL> + freeze(p); CL> spin_lock_irqsave(&p->sighand->siglock, flags); CL> signal_wake_up(p, 0); CL> spin_unlock_irqrestore(&p->sighand->siglock, flags); CL> @@ -91,7 +87,7 @@ int freeze_processes(void) CL> return todo; CL> } CL> } while(todo); CL> - CL> + CL> printk( "|\n" ); CL> BUG_ON(in_atomic()); CL> return 0; CL> @@ -106,10 +102,7 @@ void thaw_processes(void) CL> do_each_thread(g, p) { CL> if (!freezeable(p)) CL> continue; - if (p->>flags & PF_FROZEN) { - p->>flags &= ~PF_FROZEN; CL> - wake_up_process(p); CL> - } else CL> + if (!thaw_process(p)) CL> printk(KERN_INFO " Strange, %s not stopped\n", p->comm ); CL> } while_each_thread(g, p); [skip]
CL> Index: linux-2.6.12/arch/i386/kernel/signal.c CL> =================================================================== CL> --- linux-2.6.12.orig/arch/i386/kernel/signal.c 2005-06-24 20:03:23.000000000 +0000 CL> +++ linux-2.6.12/arch/i386/kernel/signal.c 2005-06-24 20:03:25.000000000 +0000 CL> @@ -608,10 +608,8 @@ int fastcall do_signal(struct pt_regs *r CL> if (!user_mode(regs)) CL> return 1; - if (current->>flags & PF_FREEZE) { CL> - refrigerator(0); CL> + if (try_to_freeze) <<<< missed ()? CL> goto no_signal;
Kirill
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |