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 0
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
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 call
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 call
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 i
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_
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 ha
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
> > try_to_freeze_
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 d
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 neces
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 show
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 d
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 sure.
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 processe
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 her
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:
> > - sho
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
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 c
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 "task_loc
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() alo
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 PROTECTED
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
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/
32 matches
Mail list logo
