public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: linux-pm@lists.linux-foundation.org
Cc: Nigel Cunningham <nigel@nigel.suspend2.net>,
	Andres Salomon <dilinger@debian.org>, Pavel Machek <pavel@ucw.cz>,
	Chris Ball <cjb@laptop.org>,
	David Woodhouse <dwmw2@infradead.org>,
	Oleg Nesterov <oleg@tv-sign.ru>
Subject: Re: Re: [RFC][PATCH -mm 2/2] Freezer: Use wait queue instead of busy looping
Date: Tue, 31 Jul 2007 12:08:40 +0200	[thread overview]
Message-ID: <200707311208.41795.rjw@sisk.pl> (raw)
In-Reply-To: <200707311139.24928.rjw@sisk.pl>

On Tuesday, 31 July 2007 11:39, Rafael J. Wysocki wrote:
> Hi,
> 
> On Tuesday, 31 July 2007 10:01, Pavel Machek wrote:
> > Hi!
> > 
> > > > > refrigerator_called is only reset after try_to_freeze_tasks() has found it
> > > > > equal to one.  There is only a small window between checking it in
> > > > > wait_event_timeout() and resetting it,
> > > > 
> > > > Yes.
> > > > 
> > > > > but then we go to send freeze requests
> > > > > to the remaining tasks and we count 'todo' from the start, so that shouldn't
> > > > > be a problem.
> > > > 
> > > > ... and we find the task which is not frozen() yet, but which has already passed
> > > > the "set condition and wakeup", increment todo, and wait for the event. If it was
> > > > the last task, we will sleep until timeout.
> > > > 
> > > > I agree, this is not fatal and unlikely, but still it is a race. I think it is
> > > > better to move this code down, after frozen_process().
> > > 
> > > OK, I see your point.  The updated patch is appended.
> > > 
> > > > (offtopic: strictly speaking, we don't even need the "refrigerator_called", we
> > > >  only need the wait_queue_head_t. try_to_freeze_tasks() just adds the "current"
> > > >  to wq at the very start of the main loop).
> > > 
> > > Hmm, yes, I think so.
> > 
> > Ok, could we just do schedule_timeout(HZ/10) or something, but when we
> > _know_ we woke someone, wakeup() that task? No new variables, keep
> > existing logic.
> 
> The logic doesn't change that much. :-)
> 
> > That should still get most of the benefits, and be two liner, no?
> 
> Well, I think we can avoid using refrigerator_called, if this is a problem, but
> the patch won't be a two liner.

To be precise, we'd need to add current to the wait queue manually, which
would require us to open code wait_event_timeout(), more or less.

Still, maybe to many things are done in this patch at a time.  I'll try to
split it into smaller steps. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

  parent reply	other threads:[~2007-07-31 10:08 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-25 12:01 [RFC][PATCH -mm 0/2] Freezer: Use wait queue instead of busy looping Rafael J. Wysocki
2007-07-25 12:03 ` [RFC][PATCH -mm 1/2] Freezer: Be more verbose Rafael J. Wysocki
2007-07-25 12:27   ` Pavel Machek
2007-07-25 12:09 ` [RFC][PATCH -mm 2/2] Freezer: Use wait queue instead of busy looping Rafael J. Wysocki
2007-07-25 12:28   ` Pavel Machek
2007-07-25 12:55     ` Rafael J. Wysocki
2007-07-25 13:29   ` Oleg Nesterov
2007-07-25 14:03     ` Rafael J. Wysocki
2007-07-25 14:24       ` Oleg Nesterov
2007-07-26 12:24         ` Rafael J. Wysocki
2007-07-26 12:43           ` Rafael J. Wysocki
2007-07-31  8:01           ` Pavel Machek
2007-07-31  9:39             ` Rafael J. Wysocki
2007-07-31 10:00               ` Pavel Machek
2007-07-31 10:17                 ` Rafael J. Wysocki
2007-07-31 10:08               ` Rafael J. Wysocki [this message]
2007-07-31 10:02                 ` Pavel Machek
2007-07-31 22:25                   ` [RFC][PATCH -mm 0/3] Freezer: Use wait queue instead of busy looping (updated) Rafael J. Wysocki
2007-07-31 22:26                     ` [RFC][PATCH -mm 1/3] Freezer: Use wait queue instead of busy looping Rafael J. Wysocki
2007-08-01  7:59                       ` Pavel Machek
2007-07-31 22:28                     ` [RFC][PATCH -mm 2/3] Freezer: Measure the time of freezing tasks Rafael J. Wysocki
2007-08-01  8:28                       ` Pavel Machek
2007-07-31 22:29                     ` [RFC][PATCH -mm 3/3] Freezer: Replace the timeout Rafael J. Wysocki
2007-08-01  8:31                       ` Pavel Machek
2007-08-01 10:43                         ` Rafael J. Wysocki
2007-08-05 21:37                           ` Pavel Machek
2007-08-05 22:38                             ` Rafael J. Wysocki
2007-08-05 22:53                               ` 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=200707311208.41795.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=cjb@laptop.org \
    --cc=dilinger@debian.org \
    --cc=dwmw2@infradead.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=nigel@nigel.suspend2.net \
    --cc=oleg@tv-sign.ru \
    --cc=pavel@ucw.cz \
    /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