public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nigel Cunningham <ncunningham@cyclades.com>
To: Christoph Lameter <christoph@lameter.com>
Cc: Pavel Machek <pavel@ucw.cz>, Andrew Morton <akpm@osdl.org>,
	Linus Torvalds <torvalds@osdl.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/4] Task notifier: Implement todo list in task_struct
Date: Tue, 09 Aug 2005 13:51:36 +1000	[thread overview]
Message-ID: <1123559495.4370.87.camel@localhost> (raw)
In-Reply-To: <Pine.LNX.4.62.0508081858040.32489@graphe.net>

Hi.

On Tue, 2005-08-09 at 12:01, Christoph Lameter wrote:
> On Tue, 9 Aug 2005, Nigel Cunningham wrote:
> 
> > Just to let you know that I have it working with Suspend2. One thing I
> > am concerned about is that we still need a way of determining whether a
> > process has been signalled but not yet frozen. At the moment you just
> > check p->todo, but if/when other functionality begins to use the todo
> > list, this will be unreliable.
> > 
> 
> No it wont. A process that has notifications to process should do that 
> before being put into the freezer. Only after the notification list is 
> empty will the process be notified and as long as the notification is 
> pending no second notification should happen.

Sorry I wasn't clear. I was thinking of the case where a broken process
doesn't process its todo list. (May it never be, but still...). How do
we find out which one is broken? We need to traverse the todo list of
every process, checking for outstanding freeze requests.

Your reply leads me to another issue. It seems to me that you shouldn't
wait until the todo list is empty before putting the freeze request on
the todo list. If the todo list is ever used for something where it
becomes hot, you might never see the todo list empty before your
timeout, and freezing will be unreliable. Even if you do see it empty
and insert your freeze request, it might be that more work is added
afterwards, so you may as well just add the request whether or not the
queue is empty.

Of course if you address this, you then have the problem that the
calling routine hammers on routines until they enter the fridge, you can
end up with multiple freeze requests per process. That could be
addressed by splitting it into two loops - the first adds the todo work
for each thread to be frozen, and the second wakes them until they
process the notification. I reckon that we shouldn't need to wake them
repeatedly like this, but I've never taken the time to see why a process
could need multiple wakeups. (I believe it's not bogus).

> > I'm happy to supply the patches I'm using if you want to compare. (I
> > retained the other freezer improvements, so it wouldn't just be bug
> > fixes against your patches).
> 
> I am not sure how to sort that out. I guess post the current patches that 
> you got and then we see how to continue from there.

Will do shortly. Just have to go talk to my boss first :> (Separate
issue).

Nigel
-- 
Evolution.
Enumerate the requirements.
Consider the interdependencies.
Calculate the probabilities.


  reply	other threads:[~2005-08-09  3:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200507260340.j6Q3eoGh029135@shell0.pdx.osdl.net>
2005-07-28  3:30 ` Alternative to TIF_FREEZE -> a notifier in the task_struct? Christoph Lameter
2005-07-28  7:41   ` Pavel Machek
2005-07-28 15:05     ` Christoph Lameter
2005-07-28 19:34       ` Pavel Machek
2005-07-28 19:49         ` Christoph Lameter
2005-07-28 19:54         ` [PATCH 1/4] Task notifier: Allow the removal of a notifier from the notifier handler Christoph Lameter
2005-07-28 19:55           ` [PATCH 2/4] Task notifier: Implement todo list in task_struct Christoph Lameter
2005-07-28 19:57             ` [PATCH 3/4] Task notifier: Make suspend code SMP safe using the todo list in the task struct Christoph Lameter
2005-07-28 19:59               ` [PATCH 4/4] Task notifier: s/try_to_freeze/try_todo_list/ in some drivers [optional] Christoph Lameter
2005-07-28 21:27             ` [PATCH 2/4] Task notifier: Implement todo list in task_struct Pavel Machek
2005-07-28 21:32               ` Pavel Machek
2005-07-28 21:57                 ` Christoph Lameter
2005-07-28 22:12                   ` Pavel Machek
2005-07-28 23:02                     ` Christoph Lameter
2005-07-28 23:27                       ` Andrew Morton
2005-07-29  0:19                         ` Christoph Lameter
2005-08-09  1:32                   ` Nigel Cunningham
2005-08-09  2:01                     ` Christoph Lameter
2005-08-09  3:51                       ` Nigel Cunningham [this message]
2005-08-09  4:21                         ` Nigel Cunningham
2005-08-09 17:05                         ` Christoph Lameter

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=1123559495.4370.87.camel@localhost \
    --to=ncunningham@cyclades.com \
    --cc=akpm@osdl.org \
    --cc=christoph@lameter.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --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