linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Christoph Lameter <christoph@lameter.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	raybry@engr.sgi.com, torvalds@osdl.org
Subject: Re: [RFC] Fix SMP brokenness for PF_FREEZE and make freezing usable for other purposes
Date: Sun, 26 Jun 2005 04:30:54 +0200	[thread overview]
Message-ID: <20050626023053.GA2871@atrey.karlin.mff.cuni.cz> (raw)
In-Reply-To: <Pine.LNX.4.62.0506242311220.7971@graphe.net>

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.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

  reply	other threads:[~2005-06-26  2:30 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-06-24 20:20 Christoph Lameter
2005-06-24 22:25 ` Nigel Cunningham
2005-06-25  2:51 ` Pavel Machek
2005-06-25  4:31   ` Christoph Lameter
2005-06-25  4:46     ` Nigel Cunningham
2005-06-25 22:37     ` Pavel Machek
2005-06-25  6:13   ` Christoph Lameter
2005-06-26  2:30     ` Pavel Machek [this message]
2005-06-26  2:55       ` Christoph Lameter
2005-06-26  3:09         ` Pavel Machek
2005-06-27  3:51           ` Christoph Lameter
2005-06-27  4:21             ` Pavel Machek
2005-06-27  4:24             ` Linus Torvalds
2005-06-27  5:53               ` Christoph Lameter
2005-06-27 14:13                 ` Pavel Machek
2005-06-27 15:13                   ` Christoph Lameter
2005-06-28  6:18                     ` Kirill Korotaev
2005-06-28  7:01                       ` Christoph Lameter
2005-06-28  7:30                         ` Kirill Korotaev
2005-06-28  8:13                           ` Kirill Korotaev
2005-06-28 14:02                             ` Christoph Lameter
2005-06-28 14:01                           ` Christoph Lameter
2005-06-28 12:47                       ` Pavel Machek
2005-07-01 17:06                         ` [SUSPEND 1/2]Replace PF_FREEZE with TIF_FREEZE Christoph Lameter
2005-07-01 17:14                           ` [SUSPEND 2/2] Replace PF_FROZEN with TASK_FROZEN Christoph Lameter
2005-06-28 21:50                 ` [RFC] Fix SMP brokenness for PF_FREEZE and make freezing usable for other purposes Ray Bryant
2005-06-28 21:54                   ` Christoph Lameter
2005-07-03 11:06                     ` Pavel Machek
2005-06-26 10:54       ` Nigel Cunningham
2005-06-25  6:18   ` Christoph Lameter
2005-06-25  7:35 ` Kirill Korotaev
2005-06-27  7:06   ` Ray Bryant
2005-06-27 13:17     ` Pavel Machek
2005-06-27 14:59       ` Ray Bryant
2005-06-27 18:05         ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20050626023053.GA2871@atrey.karlin.mff.cuni.cz \
    --to=pavel@ucw.cz \
    --cc=christoph@lameter.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=raybry@engr.sgi.com \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox