Re: [RFC] Fix SMP brokenness for PF_FREEZE and make freezing usable for other purposes

Pavel Machek
2005-06-26T02:30:54Z

Hi!

> > It includes whitespace changes and most of patch is nice cleanup that
> > should probably go in separately. (Hint hint :-).
>
> > Previous code had important property: try_to_freeze was optimized away
> > in !CONFIG_PM case. Please keep that.
> >
> > Best way is to introduce macros and cleanup the code to use the
> > macros, without actually changing any object code. That can go in very
> > fast. Then we can switch to atomic_t ... yeah I think that's
> > neccessary, but I'd like cleanups first.
>
> Here is such a patch:
>
> ---
> Cleanup patch for freezing against 2.6.12-git5.
>
> 1. Establish a simple API for process freezing defined in linux/include/sched.h:
>
> frozen(process) Check for frozen process
> freezing(process) Check if a process is being frozen
> freeze(process) Tell a process to freeze (go to refrigerator)
> thaw_process(process) Restart process
> frozen_process(process) Process is frozen now
>
> 2. Remove all references to PF_FREEZE and PF_FROZEN from all
> kernel sources except sched.h
>
> 3. Fix numerous locations where try_to_freeze is manually done by a driver
>
> 4. Remove the argument that is no longer necessary from two function
> calls.

Can you just keep the argument? Rename it to int unused or whatever,
but if you do it, it stays backwards-compatible (and smaller patch,
too).

> 5. Some whitespace cleanup
>
> 6. Clear potential race in refrigerator (provides an open window of PF_FREEZE
> cleared before setting PF_FROZEN, recalc_sigpending does not check
> PF_FROZEN).
>
> This patch does not address the problem of freeze_processes() violating the rule
> that a task may only modify its own flags by setting PF_FREEZE. This is not clean
> in an SMP environment. freeze(process) is therefore not SMP safe!
>
> Signed-off-by: Christoph Lameter <christoph@lameter.com>

Looks mostly good but:

> Index: linux-2.6.12/arch/i386/kernel/signal.c
> ===================================================================
> --- linux-2.6.12.orig/arch/i386/kernel/signal.c 2005-06-25 05:01:26.000000000 +0000
> +++ linux-2.6.12/arch/i386/kernel/signal.c 2005-06-25 05:01:28.000000000 +0000
> @@ -608,10 +608,8 @@ int fastcall do_signal(struct pt_regs *r
> if (!user_mode(regs))
> return 1;
>
> - if (current->flags & PF_FREEZE) {
> - refrigerator(0);
> + if (try_to_freeze)
> goto no_signal;
> - }
>

This is not good. Missing ().

Pavel
--
Boycott Kodak -- for their patent abuse against Java.