| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <200702192221.35219.rjw@sisk.pl>
Date: Mon, 19 Feb 2007 22:21:27 +0100
From: "Rafael J. Wysocki" <rjw@...k.pl>
To: Oleg Nesterov <oleg@...sign.ru>
Cc: ego@...ibm.com, akpm@...l.org, paulmck@...ibm.com, mingo@...e.hu,
vatsa@...ibm.com, dipankar@...ibm.com,
venkatesh.pallipadi@...el.com, linux-kernel@...r.kernel.org,
Pavel Machek <pavel@....cz>
Subject: Re: freezer problems
On Monday, 19 February 2007 21:23, Oleg Nesterov wrote:
> On 02/19, Rafael J. Wysocki wrote:
> >
> > On Sunday, 18 February 2007 23:01, Oleg Nesterov wrote:
> > > > --- linux-2.6.20-mm2.orig/include/asm-i386/thread_info.h 2007-02-18 19:49:34.000000000 +0100
> > > > +++ linux-2.6.20-mm2/include/asm-i386/thread_info.h 2007-02-18 19:50:37.000000000 +0100
> > > > @@ -135,6 +135,7 @@ static inline struct thread_info *curren
> > > > #define TIF_IO_BITMAP 18 /* uses I/O bitmap */
> > > > #define TIF_FREEZE 19 /* is freezing for suspend */
> > > > #define TIF_FORCED_TF 20 /* true if TF in eflags artificially */
> > > > +#define TIF_FREEZER_SKIP 21 /* task freezer should not count us */
> > >
> > > Do we need to put this flag into thread_info? It is always modified by
> > > "current", so it could live in task_struct->flags instead.
> >
> > I thought we were running low on the task_struct->flags bits. :-)
>
> Didn't think about that :)
Seriously, I'm not sure. There are 23 PF_* flags already defined, while
for example on x86_64 there are 17 TIF_* flags defined which is not that
much better.
> > Apart from this, we may need to set it from somewhere else in the future.
>
> I doubt. In any case, since you provided the nice helpers, it would be very
> easy to convert from thread to process flags. My main concern is that we
> have 24 include/asm-*/thread_info.h files, but only 1 include/linux/sched.h.
> It seems more easy to start with PF_ flags at first.
OK
> > @@ -1393,7 +1394,10 @@ long do_fork(unsigned long clone_flags,
> >
> > if (clone_flags & CLONE_VFORK) {
> > + freezer_do_not_count(current);
> > wait_for_completion(&vfork);
> > + try_to_freeze();
> > + freezer_count(current);
>
> freezer_do_not_count() implies that we must do try_to_freeze() later, I'd
> suggest to shift try_to_freeze() into freezer_count(). Actually, I think that
> freezer_do_not_count/freezer_count should be "(void)", like try_to_freeze().
> IOW,
>
> freezer_do_not_count()
> ... sleep in 'D' state ...
> freezer_count()
>
> means that current doesn't hold any "important" locks, may be considered as
> frozen, it can do nothing except enter refrigerator if it gets CPU.
>
> (Please feel free to ignore, this is a matter of taste of course).
Well, if we use a PF_* flag for that, it's also a matter of correctness (only
current should be able to set its flags).
> > @@ -199,6 +189,10 @@ static void thaw_tasks(int thaw_user_spa
> >
> > read_lock(&tasklist_lock);
> > do_each_thread(g, p) {
> > + if (freezer_should_skip(p))
> > + cancel_freezing(p);
> > + } while_each_thread(g, p);
> > + do_each_thread(g, p) {
> > if (!freezeable(p))
> > continue;
>
> Any reason for 2 separate do_each_thread() loops ?
Yes. If there is a "freeze" request pending for the vfork parent (TIF_FREEZE
set), we have to cancel it before the child is unfrozen, since otherwise the
parent may go freezing after we try to reset PF_FROZEN for it.
> I think this patch is correct, but I still can't convince myself I really
> understand freezer :)
Oh, that takes time. It took me a year or so. ;-)
Here's the updated patch. It hasn't been tested yet, but at least it compiles
(on x86_64).
include/linux/sched.h | 1 +
include/linux/freezer.h | 30 ++++++++++++++++++++++++++++--
kernel/fork.c | 3 +++
kernel/power/process.c | 27 ++++++++++++---------------
4 files changed, 44 insertions(+), 17 deletions(-)
Index: linux-2.6.20-mm2/include/linux/sched.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/sched.h
+++ linux-2.6.20-mm2/include/linux/sched.h
@@ -1189,6 +1189,7 @@ static inline void put_task_struct(struc
#define PF_SPREAD_SLAB 0x02000000 /* Spread some slab caches over cpuset */
#define PF_MEMPOLICY 0x10000000 /* Non-default NUMA mempolicy */
#define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */
+#define PF_FREEZER_SKIP 0x40000000 /* Freezer should not count it as freezeable */
/*
* Only the _current_ task can read/write to tsk->flags, but other
Index: linux-2.6.20-mm2/include/linux/freezer.h
===================================================================
--- linux-2.6.20-mm2.orig/include/linux/freezer.h
+++ linux-2.6.20-mm2/include/linux/freezer.h
@@ -71,7 +71,31 @@ static inline int try_to_freeze(void)
return 0;
}
-extern void thaw_some_processes(int all);
+/*
+ * Tell the freezer not to count current task as freezeable
+ */
+static inline void freezer_do_not_count(void)
+{
+ current->flags |= PF_FREEZER_SKIP;
+}
+
+/*
+ * Try to freeze the current task and tell the freezer to count it as freezeable
+ * again
+ */
+static inline void freezer_count(void)
+{
+ try_to_freeze();
+ current->flags &= ~PF_FREEZER_SKIP;
+}
+
+/*
+ * Check if the task should be counted as freezeable by the freezer
+ */
+static inline int freezer_should_skip(struct task_struct *p)
+{
+ return !!(p->flags & PF_FREEZER_SKIP);
+}
#else
static inline int frozen(struct task_struct *p) { return 0; }
@@ -86,5 +110,7 @@ static inline void thaw_processes(void)
static inline int try_to_freeze(void) { return 0; }
-
+static inline void freezer_do_not_count(void) {}
+static inline void freezer_count(void) {}
+static inline int freezer_should_skip(struct task_struct *p) { return 0; }
#endif
Index: linux-2.6.20-mm2/kernel/fork.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/fork.c
+++ linux-2.6.20-mm2/kernel/fork.c
@@ -50,6 +50,7 @@
#include <linux/taskstats_kern.h>
#include <linux/random.h>
#include <linux/ptrace.h>
+#include <linux/freezer.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -1393,7 +1394,9 @@ long do_fork(unsigned long clone_flags,
tracehook_report_clone_complete(clone_flags, nr, p);
if (clone_flags & CLONE_VFORK) {
+ freezer_do_not_count();
wait_for_completion(&vfork);
+ freezer_count();
tracehook_report_vfork_done(p, nr);
}
} else {
Index: linux-2.6.20-mm2/kernel/power/process.c
===================================================================
--- linux-2.6.20-mm2.orig/kernel/power/process.c
+++ linux-2.6.20-mm2/kernel/power/process.c
@@ -117,22 +117,12 @@ static unsigned int try_to_freeze_tasks(
cancel_freezing(p);
continue;
}
- if (is_user_space(p)) {
- if (!freeze_user_space)
- continue;
-
- /* Freeze the task unless there is a vfork
- * completion pending
- */
- if (!p->vfork_done)
- freeze_process(p);
- } else {
- if (freeze_user_space)
- continue;
+ if (is_user_space(p) == !freeze_user_space)
+ continue;
- freeze_process(p);
- }
- todo++;
+ freeze_process(p);
+ if (!freezer_should_skip(p))
+ todo++;
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
yield(); /* Yield is okay here */
@@ -199,6 +189,13 @@ static void thaw_tasks(int thaw_user_spa
read_lock(&tasklist_lock);
do_each_thread(g, p) {
+ if (is_user_space(p) == !thaw_user_space)
+ continue;
+
+ if (freezer_should_skip(p))
+ cancel_freezing(p);
+ } while_each_thread(g, p);
+ do_each_thread(g, p) {
if (!freezeable(p))
continue;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists