I guess I should just shut up and listen to experts... ok, just my
0.02 roubles.
Again, I can't comment things like "we just should never touch kernel",
because I don't even remotely undestand the things which actually use
freezer. I can only talk about the current implementation of freezer.
On
On Saturday, 12 May 2007 18:58, Oleg Nesterov wrote:
> On 05/12, Rafael J. Wysocki wrote:
> >
> > Ah, I see. We spawn a kernel thread from a code path that belongs to a
> > user space task and we need to call deamonize() to make it become a
> > 'real' kernel thread.
> >
> > Still, this means
On 05/12, Rafael J. Wysocki wrote:
>
> Ah, I see. We spawn a kernel thread from a code path that belongs to a
> user space task and we need to call deamonize() to make it become a
> 'real' kernel thread.
>
> Still, this means that is_user_space() may return 'true' for this thread
> before it
On Saturday, 12 May 2007 16:18, Oleg Nesterov wrote:
> On 05/12, Rafael J. Wysocki wrote:
> >
> > ... user space tasks that call deamonize() can also be frozen prematurely.
> > We didn't take this possibility into consideration before, which was
> > obviously
> > wrong.
>
> No, no, sorry for the
On 05/12, Rafael J. Wysocki wrote:
>
> Hmm, I'm not sure if the task can call try_to_freeze() after doing exit().
> May it happen?
It can, note the do_exit()->ptrace_notify(). But this doesn't matter, I think.
>From the freezer POV an exiting task has 2 "interesting" events
exit_mm()
On 05/12, Rafael J. Wysocki wrote:
>
> ... user space tasks that call deamonize() can also be frozen prematurely.
> We didn't take this possibility into consideration before, which was obviously
> wrong.
No, no, sorry for the confusion. User space tasks never call deamonize().
Kernel threads
On Saturday, 12 May 2007 12:52, Gautham R Shenoy wrote:
> On Sat, May 12, 2007 at 12:41:54PM +0200, Rafael J. Wysocki wrote:
> >
> > Still, the following scenario is possible while we're freezing users space
> > tasks:
> >
> > (1) user space task calls daemonize()
> > (2) freezer checks if this
On Sat, May 12, 2007 at 12:41:54PM +0200, Rafael J. Wysocki wrote:
>
> Still, the following scenario is possible while we're freezing users space
> tasks:
>
> (1) user space task calls daemonize()
> (2) freezer checks if this is a user space task and the test returns 'true'
> (3) task calls
On Saturday, 12 May 2007 11:01, Rafael J. Wysocki wrote:
> On Saturday, 12 May 2007 03:24, Linus Torvalds wrote:
> >
> > On Sat, 12 May 2007, Oleg Nesterov wrote:
> > >
> > > However, in my opininon THAT PATCH has nothing to do with this problem.
> > > It just improves the code that we already
On Saturday, 12 May 2007 12:13, Gautham R Shenoy wrote:
> On Sat, May 12, 2007 at 11:27:36AM +0200, Rafael J. Wysocki wrote:
> > On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
> > >
> > > But I am not sure if this is the case with suspend/hibernate, since we
> > > need to do a sys_sync()
On Sat, May 12, 2007 at 11:27:36AM +0200, Rafael J. Wysocki wrote:
> On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
> >
> > But I am not sure if this is the case with suspend/hibernate, since we
> > need to do a sys_sync() between try_freeze_tasks(FREEZE_USER_SPACE) and
> >
On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
> On Sat, May 12, 2007 at 02:01:41AM +0200, Rafael J. Wysocki wrote:
> >
> > > So you migt as well not return any value at all, since the returned value
> > > is apparently meaningless once the lock has been released.
> >
> > No, it is not
On Saturday, 12 May 2007 03:24, Linus Torvalds wrote:
>
> On Sat, 12 May 2007, Oleg Nesterov wrote:
> >
> > However, in my opininon THAT PATCH has nothing to do with this problem.
> > It just improves the code that we already have.
>
> Sure.
>
> However, I think it does it THE WRONG WAY, and
On Sat, May 12, 2007 at 02:01:41AM +0200, Rafael J. Wysocki wrote:
>
> > So you migt as well not return any value at all, since the returned value
> > is apparently meaningless once the lock has been released.
>
> No, it is not meaningless.
Right.
Agreed that the returned value might not
On Sat, May 12, 2007 at 02:01:41AM +0200, Rafael J. Wysocki wrote:
So you migt as well not return any value at all, since the returned value
is apparently meaningless once the lock has been released.
No, it is not meaningless.
Right.
Agreed that the returned value might not necessarily
On Saturday, 12 May 2007 03:24, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
However, in my opininon THAT PATCH has nothing to do with this problem.
It just improves the code that we already have.
Sure.
However, I think it does it THE WRONG WAY, and doesn't
On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
On Sat, May 12, 2007 at 02:01:41AM +0200, Rafael J. Wysocki wrote:
So you migt as well not return any value at all, since the returned value
is apparently meaningless once the lock has been released.
No, it is not meaningless.
On Sat, May 12, 2007 at 11:27:36AM +0200, Rafael J. Wysocki wrote:
On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
But I am not sure if this is the case with suspend/hibernate, since we
need to do a sys_sync() between try_freeze_tasks(FREEZE_USER_SPACE) and
On Saturday, 12 May 2007 12:13, Gautham R Shenoy wrote:
On Sat, May 12, 2007 at 11:27:36AM +0200, Rafael J. Wysocki wrote:
On Saturday, 12 May 2007 10:16, Gautham R Shenoy wrote:
But I am not sure if this is the case with suspend/hibernate, since we
need to do a sys_sync() between
On Saturday, 12 May 2007 11:01, Rafael J. Wysocki wrote:
On Saturday, 12 May 2007 03:24, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
However, in my opininon THAT PATCH has nothing to do with this problem.
It just improves the code that we already have.
On Sat, May 12, 2007 at 12:41:54PM +0200, Rafael J. Wysocki wrote:
Still, the following scenario is possible while we're freezing users space
tasks:
(1) user space task calls daemonize()
(2) freezer checks if this is a user space task and the test returns 'true'
(3) task calls exit_mm()
On Saturday, 12 May 2007 12:52, Gautham R Shenoy wrote:
On Sat, May 12, 2007 at 12:41:54PM +0200, Rafael J. Wysocki wrote:
Still, the following scenario is possible while we're freezing users space
tasks:
(1) user space task calls daemonize()
(2) freezer checks if this is a user
On 05/12, Rafael J. Wysocki wrote:
... user space tasks that call deamonize() can also be frozen prematurely.
We didn't take this possibility into consideration before, which was obviously
wrong.
No, no, sorry for the confusion. User space tasks never call deamonize().
Kernel threads call
On 05/12, Rafael J. Wysocki wrote:
Hmm, I'm not sure if the task can call try_to_freeze() after doing exit().
May it happen?
It can, note the do_exit()-ptrace_notify(). But this doesn't matter, I think.
From the freezer POV an exiting task has 2 interesting events
exit_mm() -mm
On Saturday, 12 May 2007 16:18, Oleg Nesterov wrote:
On 05/12, Rafael J. Wysocki wrote:
... user space tasks that call deamonize() can also be frozen prematurely.
We didn't take this possibility into consideration before, which was
obviously
wrong.
No, no, sorry for the confusion.
On 05/12, Rafael J. Wysocki wrote:
Ah, I see. We spawn a kernel thread from a code path that belongs to a
user space task and we need to call deamonize() to make it become a
'real' kernel thread.
Still, this means that is_user_space() may return 'true' for this thread
before it calls
On Saturday, 12 May 2007 18:58, Oleg Nesterov wrote:
On 05/12, Rafael J. Wysocki wrote:
Ah, I see. We spawn a kernel thread from a code path that belongs to a
user space task and we need to call deamonize() to make it become a
'real' kernel thread.
Still, this means that
I guess I should just shut up and listen to experts... ok, just my
0.02 roubles.
Again, I can't comment things like we just should never touch kernel,
because I don't even remotely undestand the things which actually use
freezer. I can only talk about the current implementation of freezer.
On
On Sat, 12 May 2007, Oleg Nesterov wrote:
>
> However, in my opininon THAT PATCH has nothing to do with this problem.
> It just improves the code that we already have.
Sure.
However, I think it does it THE WRONG WAY, and doesn't actually fix the
much deeper problems with the freezer, as
On Saturday, 12 May 2007 02:08, Linus Torvalds wrote:
>
> On Sat, 12 May 2007, Oleg Nesterov wrote:
> >
> > things change, ->mm is not stable if the kernel thread does use_mm/unuse_mm.
>
> ->mm is not stable *regardless*!
>
> Trivial examples:
> - kernel thread does execve()
> - user thread
On 05/12, Oleg Nesterov wrote:
>
> Do we need freezer? Should we freeze kernel threads? I can't judge. I tried
> to read a long thread about suspend, and failed to understand it.
>
> I personally think we can simplify things if CPU-hotplug use freezer, at
> least.
Just a small example,
On 05/11, Linus Torvalds wrote:
>
> On Sat, 12 May 2007, Oleg Nesterov wrote:
> >
> > things change, ->mm is not stable if the kernel thread does use_mm/unuse_mm.
>
> ->mm is not stable *regardless*!
>
> Trivial examples:
> - kernel thread does execve()
> - user thread does exit().
Yes
On Saturday, 12 May 2007 01:25, Andrew Morton wrote:
> On Sat, 12 May 2007 01:22:06 +0200
> "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
>
> > On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
> > >
> > > On Fri, 11 May 2007, Rafael J. Wysocki wrote:
> > > >
> > > > For user space
On Sat, 12 May 2007, Oleg Nesterov wrote:
>
> things change, ->mm is not stable if the kernel thread does use_mm/unuse_mm.
->mm is not stable *regardless*!
Trivial examples:
- kernel thread does execve()
- user thread does exit().
The use "use_mm()" and "unuse_mm()" things are total red
I hope Rafael will correct me if I am wrong,
On 05/12, Oleg Nesterov wrote:
>
> On 05/11, Linus Torvalds wrote:
> >
> > On Sat, 12 May 2007, Oleg Nesterov wrote:
> > >
> > > without task_lock() we can see "p->mm != NULL" but not PF_BORROWED_MM.
> >
> > Let me explain it one more time:
> > -
On Saturday, 12 May 2007 01:29, Linus Torvalds wrote:
>
> On Sat, 12 May 2007, Rafael J. Wysocki wrote:
> >
> > We use this function (ie. kernel/power/process.c:is_user_space()) to
> > distinguish kernel threads from user space processes. Therefore we make it
> > always return true for user
On 05/11, Linus Torvalds wrote:
>
> On Sat, 12 May 2007, Oleg Nesterov wrote:
> >
> > without task_lock() we can see "p->mm != NULL" but not PF_BORROWED_MM.
>
> Let me explain it one more time:
> - shouldn't the *caller* protect this?
>
> Afaik, there's two situations:
> - either things don't
On Sat, 12 May 2007, Oleg Nesterov wrote:
>
> without task_lock() we can see "p->mm != NULL" but not PF_BORROWED_MM.
Let me explain it one more time:
- shouldn't the *caller* protect this?
Afaik, there's two situations:
- either things don't change (in which case you don't need locking at
On Sat, 12 May 2007, Rafael J. Wysocki wrote:
>
> We use this function (ie. kernel/power/process.c:is_user_space()) to
> distinguish kernel threads from user space processes. Therefore we make it
> always return true for user space processes and always return false for kernel
> threads. In the
On Sat, 12 May 2007 01:22:06 +0200
"Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
> On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
> >
> > On Fri, 11 May 2007, Rafael J. Wysocki wrote:
> > >
> > > For user space processes this condition is always true.
> > >
> > > For kernel threads:
>
On 05/11, Linus Torvalds wrote:
>
> On Fri, 11 May 2007, Rafael J. Wysocki wrote:
> >
> > Therefore, by taking the task_lock() here we make sure that the condition
> > is alyways false when we check it for kernel threads.
>
> Why *test* it then and return anything?
>
> Why not just doa
On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
>
> On Fri, 11 May 2007, Rafael J. Wysocki wrote:
> >
> > For user space processes this condition is always true.
> >
> > For kernel threads:
> > (1) the change of tsk->mm from NULL to a nonzero value is only made in
> > fs/aio.c:use_mm()
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
>
> For user space processes this condition is always true.
>
> For kernel threads:
> (1) the change of tsk->mm from NULL to a nonzero value is only made in
> fs/aio.c:use_mm() along with the setting of PF_BORROWED_MM under
> the task_lock(),
> (2)
On Friday, 11 May 2007 21:39, Andrew Morton wrote:
> On Fri, 11 May 2007 00:36:25 +0200
> "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
>
> > The reading of PF_BORROWED_MM in is_user_space() without task_lock() is
> > racy.
> > Fix it.
> >
> > Signed-off-by: Rafael J. Wysocki <[EMAIL
On 05/11, Andrew Morton wrote:
>
> On Fri, 11 May 2007 00:36:25 +0200
> "Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
>
> > static inline int is_user_space(struct task_struct *p)
> > {
> > - return p->mm && !(p->flags & PF_BORROWED_MM);
> > + int ret;
> > +
> > + task_lock(p);
> > +
On Fri, 11 May 2007 00:36:25 +0200
"Rafael J. Wysocki" <[EMAIL PROTECTED]> wrote:
> The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
> Fix it.
>
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> Acked-by: Pavel Machek <[EMAIL PROTECTED]>
> ---
>
From: Rafael J. Wysocki <[EMAIL PROTECTED]>
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
Acked-by: Pavel Machek <[EMAIL PROTECTED]>
---
kernel/power/process.c |8 +++-
1 file changed, 7
From: Rafael J. Wysocki [EMAIL PROTECTED]
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by: Pavel Machek [EMAIL PROTECTED]
---
kernel/power/process.c |8 +++-
1 file changed, 7
On Fri, 11 May 2007 00:36:25 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by: Pavel Machek [EMAIL PROTECTED]
---
kernel/power/process.c |
On 05/11, Andrew Morton wrote:
On Fri, 11 May 2007 00:36:25 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
static inline int is_user_space(struct task_struct *p)
{
- return p-mm !(p-flags PF_BORROWED_MM);
+ int ret;
+
+ task_lock(p);
+ ret = p-mm !(p-flags
On Friday, 11 May 2007 21:39, Andrew Morton wrote:
On Fri, 11 May 2007 00:36:25 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is
racy.
Fix it.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by:
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
For user space processes this condition is always true.
For kernel threads:
(1) the change of tsk-mm from NULL to a nonzero value is only made in
fs/aio.c:use_mm() along with the setting of PF_BORROWED_MM under
the task_lock(),
(2) the
On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
For user space processes this condition is always true.
For kernel threads:
(1) the change of tsk-mm from NULL to a nonzero value is only made in
fs/aio.c:use_mm() along with the
On 05/11, Linus Torvalds wrote:
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
Therefore, by taking the task_lock() here we make sure that the condition
is alyways false when we check it for kernel threads.
Why *test* it then and return anything?
Why not just doa task_lock(p);
On Sat, 12 May 2007 01:22:06 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
For user space processes this condition is always true.
For kernel threads:
(1) the change of
On Sat, 12 May 2007, Rafael J. Wysocki wrote:
We use this function (ie. kernel/power/process.c:is_user_space()) to
distinguish kernel threads from user space processes. Therefore we make it
always return true for user space processes and always return false for kernel
threads. In the
On Sat, 12 May 2007, Oleg Nesterov wrote:
without task_lock() we can see p-mm != NULL but not PF_BORROWED_MM.
Let me explain it one more time:
- shouldn't the *caller* protect this?
Afaik, there's two situations:
- either things don't change (in which case you don't need locking at
On 05/11, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
without task_lock() we can see p-mm != NULL but not PF_BORROWED_MM.
Let me explain it one more time:
- shouldn't the *caller* protect this?
Afaik, there's two situations:
- either things don't change (in
On Saturday, 12 May 2007 01:29, Linus Torvalds wrote:
On Sat, 12 May 2007, Rafael J. Wysocki wrote:
We use this function (ie. kernel/power/process.c:is_user_space()) to
distinguish kernel threads from user space processes. Therefore we make it
always return true for user space
I hope Rafael will correct me if I am wrong,
On 05/12, Oleg Nesterov wrote:
On 05/11, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
without task_lock() we can see p-mm != NULL but not PF_BORROWED_MM.
Let me explain it one more time:
- shouldn't the *caller*
On Sat, 12 May 2007, Oleg Nesterov wrote:
things change, -mm is not stable if the kernel thread does use_mm/unuse_mm.
-mm is not stable *regardless*!
Trivial examples:
- kernel thread does execve()
- user thread does exit().
The use use_mm() and unuse_mm() things are total red herrings.
On Saturday, 12 May 2007 01:25, Andrew Morton wrote:
On Sat, 12 May 2007 01:22:06 +0200
Rafael J. Wysocki [EMAIL PROTECTED] wrote:
On Saturday, 12 May 2007 00:56, Linus Torvalds wrote:
On Fri, 11 May 2007, Rafael J. Wysocki wrote:
For user space processes this condition is
On 05/11, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
things change, -mm is not stable if the kernel thread does use_mm/unuse_mm.
-mm is not stable *regardless*!
Trivial examples:
- kernel thread does execve()
- user thread does exit().
Yes sure. Quoting
On 05/12, Oleg Nesterov wrote:
Do we need freezer? Should we freeze kernel threads? I can't judge. I tried
to read a long thread about suspend, and failed to understand it.
I personally think we can simplify things if CPU-hotplug use freezer, at
least.
Just a small example,
On Saturday, 12 May 2007 02:08, Linus Torvalds wrote:
On Sat, 12 May 2007, Oleg Nesterov wrote:
things change, -mm is not stable if the kernel thread does use_mm/unuse_mm.
-mm is not stable *regardless*!
Trivial examples:
- kernel thread does execve()
- user thread does exit().
On Sat, 12 May 2007, Oleg Nesterov wrote:
However, in my opininon THAT PATCH has nothing to do with this problem.
It just improves the code that we already have.
Sure.
However, I think it does it THE WRONG WAY, and doesn't actually fix the
much deeper problems with the freezer, as shown
On Fri 2007-02-23 11:18:06, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[EMAIL PROTECTED]>
>
> The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
> Fix it.
>
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
ACK.
--
(english)
On Fri 2007-02-23 11:18:06, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki [EMAIL PROTECTED]
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
ACK.
--
(english)
From: Rafael J. Wysocki <[EMAIL PROTECTED]>
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
---
kernel/power/process.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index:
From: Rafael J. Wysocki [EMAIL PROTECTED]
The reading of PF_BORROWED_MM in is_user_space() without task_lock() is racy.
Fix it.
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
---
kernel/power/process.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
Index:
70 matches
Mail list logo
