Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
>
> On Thursday, 2 August 2007 23:23, Oleg Nesterov wrote:
> > On 08/02, Rafael J. Wysocki wrote:
> > >
> > > On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
> > > > On 08/02, Rafael J. Wysocki wrote:
> > > > >
> > > > > @@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
> > > > >
> > > > > end_time = jiffies + TIMEOUT;
> > > > > do {
> > > > > + DEFINE_WAIT(wait);
> > > > > +
> > > > > + add_wait_queue(_waitq, );
> > > >
> > > > Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
> > > > multiple wakeups from refrigerator() won't do unnecessary work,
> > >
> > > I'm not sure what you mean.
> > >
> > > Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
> > > should remove us from the queue?
> >
> > No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
> > remove us because DEFINE_WAIT() uses autoremove_wake_function().
>
> Yes, it does, but the prepare_to_wait() version would only cause current to
> become TASK_UNINTERRUPTIBLE before it sends freezing requests, the other
> differences don't seem to matter. I'm trying to understand why it would
> change
> the behavior in the way you have described.
Ugh, this is not the first time when I didn't read your patch carefully, sorry!
I missed that your patch already uses DEFINE_WAIT(), and I was confused by the
add_wait_queue() which is usually used along with DECLARE_WAITQUEUE().
Oleg.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 23:23, Oleg Nesterov wrote:
> On 08/02, Rafael J. Wysocki wrote:
> >
> > On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
> > > On 08/02, Rafael J. Wysocki wrote:
> > > >
> > > > @@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
> > > >
> > > > end_time = jiffies + TIMEOUT;
> > > > do {
> > > > + DEFINE_WAIT(wait);
> > > > +
> > > > + add_wait_queue(_waitq, );
> > >
> > > Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
> > > multiple wakeups from refrigerator() won't do unnecessary work,
> >
> > I'm not sure what you mean.
> >
> > Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
> > should remove us from the queue?
>
> No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
> remove us because DEFINE_WAIT() uses autoremove_wake_function().
Yes, it does, but the prepare_to_wait() version would only cause current to
become TASK_UNINTERRUPTIBLE before it sends freezing requests, the other
differences don't seem to matter. I'm trying to understand why it would change
the behavior in the way you have described.
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
>
> On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
> > On 08/02, Rafael J. Wysocki wrote:
> > >
> > > @@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
> > >
> > > end_time = jiffies + TIMEOUT;
> > > do {
> > > + DEFINE_WAIT(wait);
> > > +
> > > + add_wait_queue(_waitq, );
> >
> > Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
> > multiple wakeups from refrigerator() won't do unnecessary work,
>
> I'm not sure what you mean.
>
> Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
> should remove us from the queue?
No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
remove us because DEFINE_WAIT() uses autoremove_wake_function().
Oleg.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
> On 08/02, Rafael J. Wysocki wrote:
> >
> > @@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
> >
> > end_time = jiffies + TIMEOUT;
> > do {
> > + DEFINE_WAIT(wait);
> > +
> > + add_wait_queue(_waitq, );
>
> Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
> multiple wakeups from refrigerator() won't do unnecessary work,
I'm not sure what you mean.
Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
should remove us from the queue?
> and
>
> > +
> > todo = 0;
> > read_lock(_lock);
> > do_each_thread(g, p) {
> > @@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
> > todo++;
> > } while_each_thread(g, p);
> > read_unlock(_lock);
> > - yield();/* Yield is okay here */
> > +
> > + set_current_state(TASK_UNINTERRUPTIBLE);
> > + if (todo && !list_empty_careful(_list))
> > + schedule_timeout(WAIT_TIME);
>
> we don't need to check list_empty_careful() before schedule, prepare_to_wait()
> sets TASK_UNINTERRUPTIBLE under wait_queue_head_t->lock.
Yes.
> Still, I personally agree with Pavel. Perhaps it is better to just replace
> yield() with schedule_timeout(a_bit).
Hmm, I think that we shouldn't wait if that's not necessary.
Greetings,
Rafael
--
"Premature optimization is the root of all evil." - Donald Knuth
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
>
> @@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
>
> end_time = jiffies + TIMEOUT;
> do {
> + DEFINE_WAIT(wait);
> +
> + add_wait_queue(_waitq, );
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work, and
> +
> todo = 0;
> read_lock(_lock);
> do_each_thread(g, p) {
> @@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
> todo++;
> } while_each_thread(g, p);
> read_unlock(_lock);
> - yield();/* Yield is okay here */
> +
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + if (todo && !list_empty_careful(_list))
> + schedule_timeout(WAIT_TIME);
we don't need to check list_empty_careful() before schedule, prepare_to_wait()
sets TASK_UNINTERRUPTIBLE under wait_queue_head_t->lock.
Still, I personally agree with Pavel. Perhaps it is better to just replace
yield() with schedule_timeout(a_bit).
Oleg.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 13:15, Rafael J. Wysocki wrote:
> On Thursday, 2 August 2007 01:48, Andrew Morton wrote:
> > On Wed, 1 Aug 2007 23:32:48 +0200
> > "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
> >
> > > +/*
> > > + * Used to notify try_to_freeze_tasks() that the refrigerator has been
> > > entered
> > > + * by a task.
> > > + */
> > > +static int refrigerator_called;
> >
> > this is rather icky.
>
> Well, the alternative would be to open code something like wait_event_timeout.
Still, having considered this for a while, I think it may be a good idea to
actually try this.
OK, here it goes (I hope I didn't messed up anything). Please replace the
previous version with this patch if you prefer it. :-)
Greetings,
Rafael
---
From: Rafael J. Wysocki <[EMAIL PROTECTED]>
Use the observation that try_to_freeze_tasks() need not loop while waiting for
the freezing tasks to enter the refrigerator and make it use a wait queue.
The idea is that after sending freeze requests to the tasks regarded as
freezable try_to_freeze_tasks() can go to sleep and wait until at least one task
enters the refrigerator. The first task that does it wakes up
try_to_freeze_tasks() and the procedure is repeated. If the refrigerator is not
entered by any tasks before TIMEOUT expires, the freezing of tasks fails.
This way, try_to_freeze_tasks() doesn't occupy the CPU unnecessarily when some
freezing tasks are waiting for I/O to complete.
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
kernel/power/process.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
Index: linux-2.6.23-rc1-mm2/kernel/power/process.c
===
--- linux-2.6.23-rc1-mm2.orig/kernel/power/process.c
+++ linux-2.6.23-rc1-mm2/kernel/power/process.c
@@ -19,6 +19,12 @@
*/
#define TIMEOUT(20 * HZ)
+/*
+ * Time to wait until one or more tasks enter the refrigerator after sending
+ * freeze requests to them.
+ */
+#define WAIT_TIME (HZ / 5)
+
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -43,6 +49,12 @@ static inline void frozen_process(void)
clear_freeze_flag(current);
}
+/*
+ * Wait queue head used by try_to_freeze_tasks() to wait for tasks to enter the
+ * refrigerator.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(refrigerator_waitq);
+
/* Refrigerator is place where frozen processes are stored :-). */
void refrigerator(void)
{
@@ -58,6 +70,9 @@ void refrigerator(void)
task_unlock(current);
return;
}
+
+ wake_up(_waitq);
+
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(_waitq, );
+
todo = 0;
read_lock(_lock);
do_each_thread(g, p) {
@@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(_lock);
- yield();/* Yield is okay here */
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (todo && !list_empty_careful(_list))
+ schedule_timeout(WAIT_TIME);
+ finish_wait(_waitq, );
+
if (time_after(jiffies, end_time))
break;
} while (todo);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
On Thursday, 2 August 2007 01:48, Andrew Morton wrote: > On Wed, 1 Aug 2007 23:32:48 +0200 > "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote: > > > +/* > > + * Used to notify try_to_freeze_tasks() that the refrigerator has been > > entered > > + * by a task. > > + */ > > +static int refrigerator_called; > > this is rather icky. Well, the alternative would be to open code something like wait_event_timeout. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
On Thursday, 2 August 2007 01:48, Andrew Morton wrote: On Wed, 1 Aug 2007 23:32:48 +0200 Rafael J. Wysocki [EMAIL PROTECTED] wrote: +/* + * Used to notify try_to_freeze_tasks() that the refrigerator has been entered + * by a task. + */ +static int refrigerator_called; this is rather icky. Well, the alternative would be to open code something like wait_event_timeout. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 13:15, Rafael J. Wysocki wrote:
On Thursday, 2 August 2007 01:48, Andrew Morton wrote:
On Wed, 1 Aug 2007 23:32:48 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
+/*
+ * Used to notify try_to_freeze_tasks() that the refrigerator has been
entered
+ * by a task.
+ */
+static int refrigerator_called;
this is rather icky.
Well, the alternative would be to open code something like wait_event_timeout.
Still, having considered this for a while, I think it may be a good idea to
actually try this.
modifies the patch, tests
OK, here it goes (I hope I didn't messed up anything). Please replace the
previous version with this patch if you prefer it. :-)
Greetings,
Rafael
---
From: Rafael J. Wysocki [EMAIL PROTECTED]
Use the observation that try_to_freeze_tasks() need not loop while waiting for
the freezing tasks to enter the refrigerator and make it use a wait queue.
The idea is that after sending freeze requests to the tasks regarded as
freezable try_to_freeze_tasks() can go to sleep and wait until at least one task
enters the refrigerator. The first task that does it wakes up
try_to_freeze_tasks() and the procedure is repeated. If the refrigerator is not
entered by any tasks before TIMEOUT expires, the freezing of tasks fails.
This way, try_to_freeze_tasks() doesn't occupy the CPU unnecessarily when some
freezing tasks are waiting for I/O to complete.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
kernel/power/process.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
Index: linux-2.6.23-rc1-mm2/kernel/power/process.c
===
--- linux-2.6.23-rc1-mm2.orig/kernel/power/process.c
+++ linux-2.6.23-rc1-mm2/kernel/power/process.c
@@ -19,6 +19,12 @@
*/
#define TIMEOUT(20 * HZ)
+/*
+ * Time to wait until one or more tasks enter the refrigerator after sending
+ * freeze requests to them.
+ */
+#define WAIT_TIME (HZ / 5)
+
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -43,6 +49,12 @@ static inline void frozen_process(void)
clear_freeze_flag(current);
}
+/*
+ * Wait queue head used by try_to_freeze_tasks() to wait for tasks to enter the
+ * refrigerator.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(refrigerator_waitq);
+
/* Refrigerator is place where frozen processes are stored :-). */
void refrigerator(void)
{
@@ -58,6 +70,9 @@ void refrigerator(void)
task_unlock(current);
return;
}
+
+ wake_up(refrigerator_waitq);
+
save = current-state;
pr_debug(%s entered refrigerator\n, current-comm);
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
+
todo = 0;
read_lock(tasklist_lock);
do_each_thread(g, p) {
@@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
- yield();/* Yield is okay here */
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (todo !list_empty_careful(wait.task_list))
+ schedule_timeout(WAIT_TIME);
+ finish_wait(refrigerator_waitq, wait);
+
if (time_after(jiffies, end_time))
break;
} while (todo);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work, and
+
todo = 0;
read_lock(tasklist_lock);
do_each_thread(g, p) {
@@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
- yield();/* Yield is okay here */
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (todo !list_empty_careful(wait.task_list))
+ schedule_timeout(WAIT_TIME);
we don't need to check list_empty_careful() before schedule, prepare_to_wait()
sets TASK_UNINTERRUPTIBLE under wait_queue_head_t-lock.
Still, I personally agree with Pavel. Perhaps it is better to just replace
yield() with schedule_timeout(a_bit).
Oleg.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work,
I'm not sure what you mean.
Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
should remove us from the queue?
and
+
todo = 0;
read_lock(tasklist_lock);
do_each_thread(g, p) {
@@ -189,7 +208,12 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
- yield();/* Yield is okay here */
+
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (todo !list_empty_careful(wait.task_list))
+ schedule_timeout(WAIT_TIME);
we don't need to check list_empty_careful() before schedule, prepare_to_wait()
sets TASK_UNINTERRUPTIBLE under wait_queue_head_t-lock.
Yes.
Still, I personally agree with Pavel. Perhaps it is better to just replace
yield() with schedule_timeout(a_bit).
Hmm, I think that we shouldn't wait if that's not necessary.
Greetings,
Rafael
--
Premature optimization is the root of all evil. - Donald Knuth
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work,
I'm not sure what you mean.
Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
should remove us from the queue?
No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
remove us because DEFINE_WAIT() uses autoremove_wake_function().
Oleg.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On Thursday, 2 August 2007 23:23, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work,
I'm not sure what you mean.
Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
should remove us from the queue?
No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
remove us because DEFINE_WAIT() uses autoremove_wake_function().
Yes, it does, but the prepare_to_wait() version would only cause current to
become TASK_UNINTERRUPTIBLE before it sends freezing requests, the other
differences don't seem to matter. I'm trying to understand why it would change
the behavior in the way you have described.
Greetings,
Rafael
--
Premature optimization is the root of all evil. - Donald Knuth
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping (updated)
On 08/02, Rafael J. Wysocki wrote:
On Thursday, 2 August 2007 23:23, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
On Thursday, 2 August 2007 20:40, Oleg Nesterov wrote:
On 08/02, Rafael J. Wysocki wrote:
@@ -171,6 +186,10 @@ static int try_to_freeze_tasks(int freez
end_time = jiffies + TIMEOUT;
do {
+ DEFINE_WAIT(wait);
+
+ add_wait_queue(refrigerator_waitq, wait);
Hmm. In that case I'd sugest to use prepare_to_wait(). This means that
multiple wakeups from refrigerator() won't do unnecessary work,
I'm not sure what you mean.
Do you mean that if we are TASK_UNINTERRUPTIBLE, then the first wake up
should remove us from the queue?
No, not because we are TASK_UNINTERRUPTIBLE, but yes, first wake up will
remove us because DEFINE_WAIT() uses autoremove_wake_function().
Yes, it does, but the prepare_to_wait() version would only cause current to
become TASK_UNINTERRUPTIBLE before it sends freezing requests, the other
differences don't seem to matter. I'm trying to understand why it would
change
the behavior in the way you have described.
Ugh, this is not the first time when I didn't read your patch carefully, sorry!
I missed that your patch already uses DEFINE_WAIT(), and I was confused by the
add_wait_queue() which is usually used along with DECLARE_WAITQUEUE().
Oleg.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
On Wed, 1 Aug 2007 23:32:48 +0200 "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote: > +/* > + * Used to notify try_to_freeze_tasks() that the refrigerator has been > entered > + * by a task. > + */ > +static int refrigerator_called; this is rather icky. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
From: Rafael J. Wysocki <[EMAIL PROTECTED]>
Use the observation that try_to_freeze_tasks() need not loop while waiting for
the freezing tasks to enter the refrigerator and make it use a wait queue.
The idea is that after sending freeze requests to the tasks regarded as
freezable try_to_freeze_tasks() can go to sleep and wait until at least one task
enters the refrigerator. The first task that does it wakes up
try_to_freeze_tasks() and the procedure is repeated. If the refrigerator is not
entered by any tasks before TIMEOUT expires, the freezing of tasks fails.
This way, try_to_freeze_tasks() doesn't occupy the CPU unnecessarily when some
freezing tasks are waiting for I/O to complete.
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
Acked-by: Pavel Machek <[EMAIL PROTECTED]>
---
kernel/power/process.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
Index: linux-2.6.23-rc1/kernel/power/process.c
===
--- linux-2.6.23-rc1.orig/kernel/power/process.c2007-07-31
21:58:32.0 +0200
+++ linux-2.6.23-rc1/kernel/power/process.c 2007-07-31 23:00:43.0
+0200
@@ -19,6 +19,12 @@
*/
#define TIMEOUT(20 * HZ)
+/*
+ * Time to wait until one or more tasks enter the refrigerator after sending
+ * freeze requests to them.
+ */
+#define WAIT_TIME (HZ / 5)
+
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -43,6 +49,18 @@ static inline void frozen_process(void)
clear_freeze_flag(current);
}
+/*
+ * Wait queue head used by try_to_freeze_tasks() to wait for tasks to enter the
+ * refrigerator.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(refrigerator_waitq);
+
+/*
+ * Used to notify try_to_freeze_tasks() that the refrigerator has been entered
+ * by a task.
+ */
+static int refrigerator_called;
+
/* Refrigerator is place where frozen processes are stored :-). */
void refrigerator(void)
{
@@ -58,6 +76,10 @@ void refrigerator(void)
task_unlock(current);
return;
}
+
+ refrigerator_called = 1;
+ wake_up(_waitq);
+
save = current->state;
pr_debug("%s entered refrigerator\n", current->comm);
@@ -170,6 +192,7 @@ static int try_to_freeze_tasks(int freez
unsigned int todo;
end_time = jiffies + TIMEOUT;
+ refrigerator_called = 0;
do {
todo = 0;
read_lock(_lock);
@@ -189,7 +212,16 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(_lock);
- yield();/* Yield is okay here */
+
+ if (todo) {
+ unsigned long ret;
+
+ ret = wait_event_timeout(refrigerator_waitq,
+ refrigerator_called, WAIT_TIME);
+ if (ret)
+ refrigerator_called = 0;
+ }
+
if (time_after(jiffies, end_time))
break;
} while (todo);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
From: Rafael J. Wysocki [EMAIL PROTECTED]
Use the observation that try_to_freeze_tasks() need not loop while waiting for
the freezing tasks to enter the refrigerator and make it use a wait queue.
The idea is that after sending freeze requests to the tasks regarded as
freezable try_to_freeze_tasks() can go to sleep and wait until at least one task
enters the refrigerator. The first task that does it wakes up
try_to_freeze_tasks() and the procedure is repeated. If the refrigerator is not
entered by any tasks before TIMEOUT expires, the freezing of tasks fails.
This way, try_to_freeze_tasks() doesn't occupy the CPU unnecessarily when some
freezing tasks are waiting for I/O to complete.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by: Pavel Machek [EMAIL PROTECTED]
---
kernel/power/process.c | 34 +-
1 file changed, 33 insertions(+), 1 deletion(-)
Index: linux-2.6.23-rc1/kernel/power/process.c
===
--- linux-2.6.23-rc1.orig/kernel/power/process.c2007-07-31
21:58:32.0 +0200
+++ linux-2.6.23-rc1/kernel/power/process.c 2007-07-31 23:00:43.0
+0200
@@ -19,6 +19,12 @@
*/
#define TIMEOUT(20 * HZ)
+/*
+ * Time to wait until one or more tasks enter the refrigerator after sending
+ * freeze requests to them.
+ */
+#define WAIT_TIME (HZ / 5)
+
#define FREEZER_KERNEL_THREADS 0
#define FREEZER_USER_SPACE 1
@@ -43,6 +49,18 @@ static inline void frozen_process(void)
clear_freeze_flag(current);
}
+/*
+ * Wait queue head used by try_to_freeze_tasks() to wait for tasks to enter the
+ * refrigerator.
+ */
+static DECLARE_WAIT_QUEUE_HEAD(refrigerator_waitq);
+
+/*
+ * Used to notify try_to_freeze_tasks() that the refrigerator has been entered
+ * by a task.
+ */
+static int refrigerator_called;
+
/* Refrigerator is place where frozen processes are stored :-). */
void refrigerator(void)
{
@@ -58,6 +76,10 @@ void refrigerator(void)
task_unlock(current);
return;
}
+
+ refrigerator_called = 1;
+ wake_up(refrigerator_waitq);
+
save = current-state;
pr_debug(%s entered refrigerator\n, current-comm);
@@ -170,6 +192,7 @@ static int try_to_freeze_tasks(int freez
unsigned int todo;
end_time = jiffies + TIMEOUT;
+ refrigerator_called = 0;
do {
todo = 0;
read_lock(tasklist_lock);
@@ -189,7 +212,16 @@ static int try_to_freeze_tasks(int freez
todo++;
} while_each_thread(g, p);
read_unlock(tasklist_lock);
- yield();/* Yield is okay here */
+
+ if (todo) {
+ unsigned long ret;
+
+ ret = wait_event_timeout(refrigerator_waitq,
+ refrigerator_called, WAIT_TIME);
+ if (ret)
+ refrigerator_called = 0;
+ }
+
if (time_after(jiffies, end_time))
break;
} while (todo);
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm 2/3] Freezer: Use wait queue instead of busy looping
On Wed, 1 Aug 2007 23:32:48 +0200 Rafael J. Wysocki [EMAIL PROTECTED] wrote: +/* + * Used to notify try_to_freeze_tasks() that the refrigerator has been entered + * by a task. + */ +static int refrigerator_called; this is rather icky. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
