Re: [PATCH] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon 2007-03-12 17:58:05, Paul E. McKenney wrote:
> On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> > Hi!
> >
> > > > > Looks good to me! The other kthread_should_stop() calls in
> > > > > rcutorture.c should also become
> > > > > kthread_should_top_check_freeze().
> >
> > > > Why is it useful?
> > >
> > > Because we want to avoid repeating
> > >
> > > while (!kthread_should_stop()) {
> > > try_to_freeze();
> > > ...
> > > }
> > >
> > > in many places?
> >
> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code.
> >
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
> >
> > NAK.
>
> Another problem is people doing "kthread_should_stop()" and forgetting
> about freezing. Then we continue ending up with situations where weare
> intermittently unable to freeze. In the spirit of "Rusty Scale" interface
No, in such case we end up with situation when it is impossible to
freeze, and that's very easy to debug.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
> > Do not do it, then. Confusion it causes is not worth saving one line
> > of code.
> >
> > You do less typing, but the resulting code is _less_ readable, not
> > more.
>
> Then please document it _clearly_ with the kthread code somewhere. The
> reason I brought this up is I had no idea we had to put the freezer gunk
> in all kernel thread loops and Ive been writing kernel threads for years.
This is in Doc*/power/kernel_threads.txt:
KERNEL THREADS
Freezer
Upon entering a suspended state the system will freeze all
tasks. This is done by delivering pseudosignals. This affects
kernel threads, too. To successfully freeze a kernel thread
the thread has to check for the pseudosignal and enter the
refrigerator. Code to do this looks like this:
do {
hub_events();
wait_event_interruptible(khubd_wait,
!list_empty(_event_list));
try_to_freeze();
} while (!signal_pending(current));
...do you know about better place?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
Then please document it _clearly_ with the kthread code somewhere. The
reason I brought this up is I had no idea we had to put the freezer gunk
in all kernel thread loops and Ive been writing kernel threads for years.
This is in Doc*/power/kernel_threads.txt:
KERNEL THREADS
Freezer
Upon entering a suspended state the system will freeze all
tasks. This is done by delivering pseudosignals. This affects
kernel threads, too. To successfully freeze a kernel thread
the thread has to check for the pseudosignal and enter the
refrigerator. Code to do this looks like this:
do {
hub_events();
wait_event_interruptible(khubd_wait,
!list_empty(hub_event_list));
try_to_freeze();
} while (!signal_pending(current));
...do you know about better place?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon 2007-03-12 17:58:05, Paul E. McKenney wrote:
On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
Hi!
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become
kthread_should_top_check_freeze().
Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
NAK.
Another problem is people doing kthread_should_stop() and forgetting
about freezing. Then we continue ending up with situations where weare
intermittently unable to freeze. In the spirit of Rusty Scale interface
No, in such case we end up with situation when it is impossible to
freeze, and that's very easy to debug.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
> Hi!
>
> > > > Looks good to me! The other kthread_should_stop() calls in
> > > > rcutorture.c should also become
> > > > kthread_should_top_check_freeze().
>
> > > Why is it useful?
> >
> > Because we want to avoid repeating
> >
> > while (!kthread_should_stop()) {
> > try_to_freeze();
> > ...
> > }
> >
> > in many places?
>
> Do not do it, then. Confusion it causes is not worth saving one line
> of code.
>
> You do less typing, but the resulting code is _less_ readable, not
> more.
>
> NAK.
Another problem is people doing "kthread_should_stop()" and forgetting
about freezing. Then we continue ending up with situations where we are
intermittently unable to freeze. In the spirit of "Rusty Scale" interface
design, how do we make it difficult for people to misuse this interface?
Thanx, Paul
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon, Mar 12, 2007 at 11:39:06PM +0100, Pavel Machek wrote:
Hi!
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become
kthread_should_top_check_freeze().
Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
NAK.
Another problem is people doing kthread_should_stop() and forgetting
about freezing. Then we continue ending up with situations where we are
intermittently unable to freeze. In the spirit of Rusty Scale interface
design, how do we make it difficult for people to misuse this interface?
Thanx, Paul
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 03:28:08PM +0530, Srivatsa Vaddagiri wrote: > On Tue, Mar 13, 2007 at 09:16:29AM +, Christoph Hellwig wrote: > > > Document as well in the kernel_thread() API, as I notice people still > > > use kernel_thread() some places (ex: rtasd.c in powerpc arch)? > > > > They shouldn't use kernel_thread. > > Hmm ..that needs to be documented as well then! I can easily count more > than dozen places where kernel_thread() is being used. > > I agree though that kthread is a much cleaner interface to create/destroy > threads. Well, it takes a lot of time to convert all the existing users. But I try to make sure to flame^H^H^H^H^Hcorrect everyone who tries to sneak a new user of kernel_thread in. - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 09:16:29AM +, Christoph Hellwig wrote: > > Document as well in the kernel_thread() API, as I notice people still > > use kernel_thread() some places (ex: rtasd.c in powerpc arch)? > > They shouldn't use kernel_thread. Hmm ..that needs to be documented as well then! I can easily count more than dozen places where kernel_thread() is being used. I agree though that kthread is a much cleaner interface to create/destroy threads. -- Regards, vatsa - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 08:44:11AM +0530, Srivatsa Vaddagiri wrote: > On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote: > > Then please document it _clearly_ with the kthread code somewhere. > > Document as well in the kernel_thread() API, as I notice people still > use kernel_thread() some places (ex: rtasd.c in powerpc arch)? They shouldn't use kernel_thread. - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 08:44:11AM +0530, Srivatsa Vaddagiri wrote: On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote: Then please document it _clearly_ with the kthread code somewhere. Document as well in the kernel_thread() API, as I notice people still use kernel_thread() some places (ex: rtasd.c in powerpc arch)? They shouldn't use kernel_thread. - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 09:16:29AM +, Christoph Hellwig wrote: Document as well in the kernel_thread() API, as I notice people still use kernel_thread() some places (ex: rtasd.c in powerpc arch)? They shouldn't use kernel_thread. Hmm ..that needs to be documented as well then! I can easily count more than dozen places where kernel_thread() is being used. I agree though that kthread is a much cleaner interface to create/destroy threads. -- Regards, vatsa - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 03:28:08PM +0530, Srivatsa Vaddagiri wrote: On Tue, Mar 13, 2007 at 09:16:29AM +, Christoph Hellwig wrote: Document as well in the kernel_thread() API, as I notice people still use kernel_thread() some places (ex: rtasd.c in powerpc arch)? They shouldn't use kernel_thread. Hmm ..that needs to be documented as well then! I can easily count more than dozen places where kernel_thread() is being used. I agree though that kthread is a much cleaner interface to create/destroy threads. Well, it takes a lot of time to convert all the existing users. But I try to make sure to flame^H^H^H^H^Hcorrect everyone who tries to sneak a new user of kernel_thread in. - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 10:57:16AM +0530, Gautham R Shenoy wrote:
> CPU_DEAD:
> thaw_process(p);
> kthread_stop(p);
> p = NULL;
This neednt guarantee that the thread will see the stop request before
it exits the kthread_should_stop_freeze() function. There will always
be races .. So the only safe way for a thread to know whether it is time
to exit is:
while (!kthread_should_stop_freeze()) {
if (!cpu_online(home_cpu))
goto wait_to_die;
...
}
wait_to_die:
while (!kthread_should_stop()) {
/* sleep */
}
--
Regards,
vatsa
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > >
> > > > > int kthread_should_stop(void)
> > > > > {
> > > > > if (kthread_stop_info.k == current)
> > > > > return 1;
> > > > > try_to_freeze();
> > > > > return 0;
> > > > > }
> > > >
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where
> > > > try_to_freeze()
> > > > is not safe.
> > > >
> > > > Perhaps we should introduce a new helper which does this.
> > >
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach. About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > >
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> >
> > I personally think we should do the opposite, add
> > kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
>
> Something like this, perhaps:
>
> include/linux/kthread.h |1 +
> kernel/kthread.c| 16
> kernel/rcutorture.c |5 ++---
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
> 21:58:48.0 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
> +0100
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> }
> EXPORT_SYMBOL(kthread_should_stop);
>
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now
> and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> + might_sleep();
> + if (kthread_stop_info.k == current)
> + return 1;
> +
> + try_to_freeze();
> + return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);
I would prefer to have try_to_freeze() followed by the
kthread_stop_info.k check. Something like
if (try_to_freeze())
/*some barrier ensuring all writes are completed */
if (kthread_stop_info.k == current)
return 1;
return 0;
This would be helpful in situations (atleast for cpu-hotplug)
where we want to stop a frozen thread immediately after thawing it.
Something like
CPU_DEAD:
thaw_process(p);
kthread_stop(p);
p = NULL;
Is there a problem with this line of thinking ?
thanks and regards
gautham.
--
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote: > Then please document it _clearly_ with the kthread code somewhere. Document as well in the kernel_thread() API, as I notice people still use kernel_thread() some places (ex: rtasd.c in powerpc arch)? > The reason I brought this up is I had no idea we had to put the freezer gunk > in all kernel thread loops and Ive been writing kernel threads for years. I noticed that in the Powerpc code (atleast for rtas kernel thread) here: http://lkml.org/lkml/2007/1/9/61 That was not a serious problem perhaps because process freezer was mostly used in software suspend and only those platforms supporting software suspend had to worry abt it. But now we intend to use process freezer for CPU hotplug as well, so all platforms wanting to support CPU hotplug better support process freezer! P.S : I believe kprobes is already using process freezer as well. -- Regards, vatsa - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
> Do not do it, then. Confusion it causes is not worth saving one line > of code. > > You do less typing, but the resulting code is _less_ readable, not > more. Then please document it _clearly_ with the kthread code somewhere. The reason I brought this up is I had no idea we had to put the freezer gunk in all kernel thread loops and Ive been writing kernel threads for years. Anton - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
> > > Looks good to me! The other kthread_should_stop() calls in
> > > rcutorture.c should also become
> > > kthread_should_top_check_freeze().
> > Why is it useful?
>
> Because we want to avoid repeating
>
> while (!kthread_should_stop()) {
> try_to_freeze();
> ...
> }
>
> in many places?
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
NAK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On 03/12, Rafael J. Wysocki wrote: > > On Monday, 12 March 2007 09:14, Pavel Machek wrote: > > > > Can we get better name for this function? > > Well, I took the name from the Oleg's message. Can you please suggest > something? Well, kthread_should_stop_check_freeze() is really awful, I agree :) We need something better, but I can't suggest anything. 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi,
On Monday, 12 March 2007 09:14, Pavel Machek wrote:
> Hi!
>
> > > > I personally think we should do the opposite, add
> > > > kthread_should_stop_check_freeze()
> > > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > > driver). The new helper is obviously "might_sleep()".
> > >
> > > Something like this, perhaps:
> >
> > Looks good to me! The other kthread_should_stop() calls in
> > rcutorture.c should also become kthread_should_top_check_freeze().
> >
> > Acked-by: Paul E. McKenney <[EMAIL PROTECTED]>
> >
> > > include/linux/kthread.h |1 +
> > > kernel/kthread.c| 16
> > > kernel/rcutorture.c |5 ++---
> > > 3 files changed, 19 insertions(+), 3 deletions(-)
> > >
> > > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > > ===
> > > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
> > > 21:58:48.0 +0100
> > > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
> > > +0100
> > > @@ -13,6 +13,7 @@
> > > #include
> > > #include
> > > #include
> > > +#include
> > > #include
> > >
> > > /*
> > > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> > > }
> > > EXPORT_SYMBOL(kthread_should_stop);
> > >
> > > +/**
> > > + * kthread_should_stop_check_freeze - check if the thread should return
> > > now and
> > > + * if not, check if there is a freezing request pending for it.
> > > + */
> > > +int kthread_should_stop_check_freeze(void)
> > > +{
> > > + might_sleep();
> > > + if (kthread_stop_info.k == current)
> > > + return 1;
> > > +
> > > + try_to_freeze();
> > > + return 0;
> > > +}
>
> Can we get better name for this function?
Well, I took the name from the Oleg's message. Can you please suggest
something?
> Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
> Caller can do "try_to_freeze()" as well, no?
Sure, it just eliminates one line of code.
> > > }
> > > rcu_torture_current_version++;
> > > oldbatch = cur_ops->completed();
> > > - try_to_freeze();
> > > - } while (!kthread_should_stop() && !fullstop);
> > > + } while (!kthread_should_stop_check_freeze() && !fullstop);
> > > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > > - while (!kthread_should_stop())
> > > + while (!kthread_should_stop_check_freeze())
> > > schedule_timeout_uninterruptible(1);
> > > return 0;
>
> Aha, I see, here it probably becomes handy.
>
> Actually, no... I do not see it. Why don't you simply move first
> try_to_freeze() to beggining of the loop and do
>
> - while (!kthread_should_stop()) {
> schedule_timeout_uninterruptible(1);
>try_to_freeze()
> }
Yes, but then the second loop will contain one more line of code.
Greetings,
Rafael
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
> > > I personally think we should do the opposite, add
> > > kthread_should_stop_check_freeze()
> > > or something. kthread_should_stop() is like signal_pending(), we can use
> > > it under spin_lock (and it is probably used this way by some out-of-tree
> > > driver). The new helper is obviously "might_sleep()".
> >
> > Something like this, perhaps:
>
> Looks good to me! The other kthread_should_stop() calls in
> rcutorture.c should also become kthread_should_top_check_freeze().
>
> Acked-by: Paul E. McKenney <[EMAIL PROTECTED]>
>
> > include/linux/kthread.h |1 +
> > kernel/kthread.c| 16
> > kernel/rcutorture.c |5 ++---
> > 3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> > ===
> > --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08
> > 21:58:48.0 +0100
> > +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
> > +0100
> > @@ -13,6 +13,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > /*
> > @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> > }
> > EXPORT_SYMBOL(kthread_should_stop);
> >
> > +/**
> > + * kthread_should_stop_check_freeze - check if the thread should return
> > now and
> > + * if not, check if there is a freezing request pending for it.
> > + */
> > +int kthread_should_stop_check_freeze(void)
> > +{
> > + might_sleep();
> > + if (kthread_stop_info.k == current)
> > + return 1;
> > +
> > + try_to_freeze();
> > + return 0;
> > +}
Can we get better name for this function?
Why is it useful? Caller can do "try_to_freeze()" as well, no?
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > - try_to_freeze();
> > - } while (!kthread_should_stop() && !fullstop);
> > + } while (!kthread_should_stop_check_freeze() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > - while (!kthread_should_stop())
> > + while (!kthread_should_stop_check_freeze())
> > schedule_timeout_uninterruptible(1);
> > return 0;
Aha, I see, here it probably becomes handy.
Actually, no... I do not see it. Why don't you simply move first
try_to_freeze() to beggining of the loop and do
- while (!kthread_should_stop()) {
schedule_timeout_uninterruptible(1);
try_to_freeze()
}
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
Something like this, perhaps:
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().
Acked-by: Paul E. McKenney [EMAIL PROTECTED]
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08
21:58:48.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include linux/file.h
#include linux/module.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/semaphore.h
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return
now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
Can we get better name for this function?
Why is it useful? Caller can do try_to_freeze() as well, no?
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
- try_to_freeze();
- } while (!kthread_should_stop() !fullstop);
+ } while (!kthread_should_stop_check_freeze() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
Aha, I see, here it probably becomes handy.
Actually, no... I do not see it. Why don't you simply move first
try_to_freeze() to beggining of the loop and do
- while (!kthread_should_stop()) {
schedule_timeout_uninterruptible(1);
try_to_freeze()
}
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi,
On Monday, 12 March 2007 09:14, Pavel Machek wrote:
Hi!
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
Something like this, perhaps:
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().
Acked-by: Paul E. McKenney [EMAIL PROTECTED]
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
21:58:48.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include linux/file.h
#include linux/module.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/semaphore.h
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return
now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
Can we get better name for this function?
Well, I took the name from the Oleg's message. Can you please suggest
something?
Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
Caller can do try_to_freeze() as well, no?
Sure, it just eliminates one line of code.
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
- try_to_freeze();
- } while (!kthread_should_stop() !fullstop);
+ } while (!kthread_should_stop_check_freeze() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
Aha, I see, here it probably becomes handy.
Actually, no... I do not see it. Why don't you simply move first
try_to_freeze() to beggining of the loop and do
- while (!kthread_should_stop()) {
schedule_timeout_uninterruptible(1);
try_to_freeze()
}
Yes, but then the second loop will contain one more line of code.
Greetings,
Rafael
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On 03/12, Rafael J. Wysocki wrote: On Monday, 12 March 2007 09:14, Pavel Machek wrote: Can we get better name for this function? Well, I took the name from the Oleg's message. Can you please suggest something? Well, kthread_should_stop_check_freeze() is really awful, I agree :) We need something better, but I can't suggest anything. 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Hi!
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become
kthread_should_top_check_freeze().
Why is it useful?
Because we want to avoid repeating
while (!kthread_should_stop()) {
try_to_freeze();
...
}
in many places?
Do not do it, then. Confusion it causes is not worth saving one line
of code.
You do less typing, but the resulting code is _less_ readable, not
more.
NAK.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
Do not do it, then. Confusion it causes is not worth saving one line of code. You do less typing, but the resulting code is _less_ readable, not more. Then please document it _clearly_ with the kthread code somewhere. The reason I brought this up is I had no idea we had to put the freezer gunk in all kernel thread loops and Ive been writing kernel threads for years. Anton - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Mon, Mar 12, 2007 at 05:45:24PM -0500, Anton Blanchard wrote: Then please document it _clearly_ with the kthread code somewhere. Document as well in the kernel_thread() API, as I notice people still use kernel_thread() some places (ex: rtasd.c in powerpc arch)? The reason I brought this up is I had no idea we had to put the freezer gunk in all kernel thread loops and Ive been writing kernel threads for years. I noticed that in the Powerpc code (atleast for rtas kernel thread) here: http://lkml.org/lkml/2007/1/9/61 That was not a serious problem perhaps because process freezer was mostly used in software suspend and only those platforms supporting software suspend had to worry abt it. But now we intend to use process freezer for CPU hotplug as well, so all platforms wanting to support CPU hotplug better support process freezer! P.S : I believe kprobes is already using process freezer as well. -- Regards, vatsa - 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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where
try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
Something like this, perhaps:
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
21:58:48.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include linux/file.h
#include linux/module.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/semaphore.h
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now
and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
I would prefer to have try_to_freeze() followed by the
kthread_stop_info.k check. Something like
if (try_to_freeze())
/*some barrier ensuring all writes are completed */
if (kthread_stop_info.k == current)
return 1;
return 0;
This would be helpful in situations (atleast for cpu-hotplug)
where we want to stop a frozen thread immediately after thawing it.
Something like
CPU_DEAD:
thaw_process(p);
kthread_stop(p);
p = NULL;
Is there a problem with this line of thinking ?
thanks and regards
gautham.
--
Gautham R Shenoy
Linux Technology Center
IBM India.
Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Tue, Mar 13, 2007 at 10:57:16AM +0530, Gautham R Shenoy wrote:
CPU_DEAD:
thaw_process(p);
kthread_stop(p);
p = NULL;
This neednt guarantee that the thread will see the stop request before
it exits the kthread_should_stop_freeze() function. There will always
be races .. So the only safe way for a thread to know whether it is time
to exit is:
while (!kthread_should_stop_freeze()) {
if (!cpu_online(home_cpu))
goto wait_to_die;
...
}
wait_to_die:
while (!kthread_should_stop()) {
/* sleep */
}
--
Regards,
vatsa
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
> On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > > On 03/02, Paul E. McKenney wrote:
> > > > >
> > > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > > as follows:
> > > > >
> > > > > int kthread_should_stop(void)
> > > > > {
> > > > > if (kthread_stop_info.k == current)
> > > > > return 1;
> > > > > try_to_freeze();
> > > > > return 0;
> > > > > }
> > > >
> > > > I think this is dangerous. For example, worker_thread() will probably
> > > > need some special actions after return from refrigerator. Also, a kernel
> > > > thread may check kthread_should_stop() in the place where
> > > > try_to_freeze()
> > > > is not safe.
> > > >
> > > > Perhaps we should introduce a new helper which does this.
> > >
> > > Good point -- the return value from try_to_freeze() is lost if one uses
> > > the above approach. About one third of the calls to try_to_freeze()
> > > in 2.6.20 pay attention to the return value.
> > >
> > > One approach would be to have a kthread_should_stop_nofreeze() for those
> > > cases, and let the default be to try to freeze.
> >
> > I personally think we should do the opposite, add
> > kthread_should_stop_check_freeze()
> > or something. kthread_should_stop() is like signal_pending(), we can use
> > it under spin_lock (and it is probably used this way by some out-of-tree
> > driver). The new helper is obviously "might_sleep()".
>
> Something like this, perhaps:
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().
Acked-by: Paul E. McKenney <[EMAIL PROTECTED]>
> include/linux/kthread.h |1 +
> kernel/kthread.c| 16
> kernel/rcutorture.c |5 ++---
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
> ===
> --- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
> 21:58:48.0 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
> +0100
> @@ -13,6 +13,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /*
> @@ -60,6 +61,21 @@ int kthread_should_stop(void)
> }
> EXPORT_SYMBOL(kthread_should_stop);
>
> +/**
> + * kthread_should_stop_check_freeze - check if the thread should return now
> and
> + * if not, check if there is a freezing request pending for it.
> + */
> +int kthread_should_stop_check_freeze(void)
> +{
> + might_sleep();
> + if (kthread_stop_info.k == current)
> + return 1;
> +
> + try_to_freeze();
> + return 0;
> +}
> +EXPORT_SYMBOL(kthread_should_stop_check_freeze);
> +
> static void kthread_exit_files(void)
> {
> struct fs_struct *fs;
> Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
> ===
> --- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04
> 19:44:54.0 +0100
> +++ linux-2.6.21-rc3-mm2/include/linux/kthread.h 2007-03-11
> 18:37:10.0 +0100
> @@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
> void kthread_bind(struct task_struct *k, unsigned int cpu);
> int kthread_stop(struct task_struct *k);
> int kthread_should_stop(void);
> +int kthread_should_stop_check_freeze(void);
>
> #endif /* _LINUX_KTHREAD_H */
> Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
> ===
> --- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11
> 11:39:06.0 +0100
> +++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c 2007-03-11 18:45:00.0
> +0100
> @@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> - try_to_freeze();
> - } while (!kthread_should_stop() && !fullstop);
> + } while (!kthread_should_stop_check_freeze() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> - while (!kthread_should_stop())
> + while (!kthread_should_stop_check_freeze())
> schedule_timeout_uninterruptible(1);
> return 0;
> }
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > > On 03/02, Paul E. McKenney wrote:
> > > >
> > > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > > as follows:
> > > >
> > > > int kthread_should_stop(void)
> > > > {
> > > > if (kthread_stop_info.k == current)
> > > > return 1;
> > > > try_to_freeze();
> > > > return 0;
> > > > }
> > >
> > > I think this is dangerous. For example, worker_thread() will probably
> > > need some special actions after return from refrigerator. Also, a kernel
> > > thread may check kthread_should_stop() in the place where try_to_freeze()
> > > is not safe.
> > >
> > > Perhaps we should introduce a new helper which does this.
> >
> > Good point -- the return value from try_to_freeze() is lost if one uses
> > the above approach. About one third of the calls to try_to_freeze()
> > in 2.6.20 pay attention to the return value.
> >
> > One approach would be to have a kthread_should_stop_nofreeze() for those
> > cases, and let the default be to try to freeze.
>
> I personally think we should do the opposite, add
> kthread_should_stop_check_freeze()
> or something. kthread_should_stop() is like signal_pending(), we can use
> it under spin_lock (and it is probably used this way by some out-of-tree
> driver). The new helper is obviously "might_sleep()".
Something like this, perhaps:
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.0
+0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
+
static void kthread_exit_files(void)
{
struct fs_struct *fs;
Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
===
--- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04
19:44:54.0 +0100
+++ linux-2.6.21-rc3-mm2/include/linux/kthread.h2007-03-11
18:37:10.0 +0100
@@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
int kthread_should_stop(void);
+int kthread_should_stop_check_freeze(void);
#endif /* _LINUX_KTHREAD_H */
Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11
11:39:06.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c2007-03-11 18:45:00.0
+0100
@@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops->completed();
- try_to_freeze();
- } while (!kthread_should_stop() && !fullstop);
+ } while (!kthread_should_stop_check_freeze() && !fullstop);
VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
}
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
Something like this, perhaps:
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c 2007-03-08 21:58:48.0
+0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include linux/file.h
#include linux/module.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/semaphore.h
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
+
static void kthread_exit_files(void)
{
struct fs_struct *fs;
Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
===
--- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04
19:44:54.0 +0100
+++ linux-2.6.21-rc3-mm2/include/linux/kthread.h2007-03-11
18:37:10.0 +0100
@@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
int kthread_should_stop(void);
+int kthread_should_stop_check_freeze(void);
#endif /* _LINUX_KTHREAD_H */
Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11
11:39:06.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c2007-03-11 18:45:00.0
+0100
@@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
- try_to_freeze();
- } while (!kthread_should_stop() !fullstop);
+ } while (!kthread_should_stop_check_freeze() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
}
-
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] kthread_should_stop_check_freeze (was: Re: [PATCH -mm 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread)
On Sun, Mar 11, 2007 at 06:49:08PM +0100, Rafael J. Wysocki wrote:
On Saturday, 3 March 2007 18:32, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where
try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
Something like this, perhaps:
Looks good to me! The other kthread_should_stop() calls in
rcutorture.c should also become kthread_should_top_check_freeze().
Acked-by: Paul E. McKenney [EMAIL PROTECTED]
include/linux/kthread.h |1 +
kernel/kthread.c| 16
kernel/rcutorture.c |5 ++---
3 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6.21-rc3-mm2/kernel/kthread.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/kthread.c2007-03-08
21:58:48.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/kthread.c 2007-03-11 18:32:59.0
+0100
@@ -13,6 +13,7 @@
#include linux/file.h
#include linux/module.h
#include linux/mutex.h
+#include linux/freezer.h
#include asm/semaphore.h
/*
@@ -60,6 +61,21 @@ int kthread_should_stop(void)
}
EXPORT_SYMBOL(kthread_should_stop);
+/**
+ * kthread_should_stop_check_freeze - check if the thread should return now
and
+ * if not, check if there is a freezing request pending for it.
+ */
+int kthread_should_stop_check_freeze(void)
+{
+ might_sleep();
+ if (kthread_stop_info.k == current)
+ return 1;
+
+ try_to_freeze();
+ return 0;
+}
+EXPORT_SYMBOL(kthread_should_stop_check_freeze);
+
static void kthread_exit_files(void)
{
struct fs_struct *fs;
Index: linux-2.6.21-rc3-mm2/include/linux/kthread.h
===
--- linux-2.6.21-rc3-mm2.orig/include/linux/kthread.h 2007-02-04
19:44:54.0 +0100
+++ linux-2.6.21-rc3-mm2/include/linux/kthread.h 2007-03-11
18:37:10.0 +0100
@@ -29,5 +29,6 @@ struct task_struct *kthread_create(int (
void kthread_bind(struct task_struct *k, unsigned int cpu);
int kthread_stop(struct task_struct *k);
int kthread_should_stop(void);
+int kthread_should_stop_check_freeze(void);
#endif /* _LINUX_KTHREAD_H */
Index: linux-2.6.21-rc3-mm2/kernel/rcutorture.c
===
--- linux-2.6.21-rc3-mm2.orig/kernel/rcutorture.c 2007-03-11
11:39:06.0 +0100
+++ linux-2.6.21-rc3-mm2/kernel/rcutorture.c 2007-03-11 18:45:00.0
+0100
@@ -540,10 +540,9 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
- try_to_freeze();
- } while (!kthread_should_stop() !fullstop);
+ } while (!kthread_should_stop_check_freeze() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
- while (!kthread_should_stop())
+ while (!kthread_should_stop_check_freeze())
schedule_timeout_uninterruptible(1);
return 0;
}
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Sat, Mar 03, 2007 at 08:32:40PM +0300, Oleg Nesterov wrote: > I personally think we should do the opposite, add > kthread_should_stop_check_freeze() > or something. kthread_should_stop() is like signal_pending(), we can use > it under spin_lock (and it is probably used this way by some out-of-tree > driver). The new helper is obviously "might_sleep()". Yes, I agree. It helps the caller explicitly know that this function -can- freeze. -- Regards, vatsa - 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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On 03/02, Paul E. McKenney wrote:
>
> On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> > On 03/02, Paul E. McKenney wrote:
> > >
> > > One way to embed try_to_freeze() into kthread_should_stop() might be
> > > as follows:
> > >
> > > int kthread_should_stop(void)
> > > {
> > > if (kthread_stop_info.k == current)
> > > return 1;
> > > try_to_freeze();
> > > return 0;
> > > }
> >
> > I think this is dangerous. For example, worker_thread() will probably
> > need some special actions after return from refrigerator. Also, a kernel
> > thread may check kthread_should_stop() in the place where try_to_freeze()
> > is not safe.
> >
> > Perhaps we should introduce a new helper which does this.
>
> Good point -- the return value from try_to_freeze() is lost if one uses
> the above approach. About one third of the calls to try_to_freeze()
> in 2.6.20 pay attention to the return value.
>
> One approach would be to have a kthread_should_stop_nofreeze() for those
> cases, and let the default be to try to freeze.
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously "might_sleep()".
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On 03/02, Paul E. McKenney wrote:
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
I personally think we should do the opposite, add
kthread_should_stop_check_freeze()
or something. kthread_should_stop() is like signal_pending(), we can use
it under spin_lock (and it is probably used this way by some out-of-tree
driver). The new helper is obviously might_sleep().
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Sat, Mar 03, 2007 at 08:32:40PM +0300, Oleg Nesterov wrote: I personally think we should do the opposite, add kthread_should_stop_check_freeze() or something. kthread_should_stop() is like signal_pending(), we can use it under spin_lock (and it is probably used this way by some out-of-tree driver). The new helper is obviously might_sleep(). Yes, I agree. It helps the caller explicitly know that this function -can- freeze. -- Regards, vatsa - 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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Saturday, 3 March 2007 00:33, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> >
> > int kthread_should_stop(void)
> > {
> > if (kthread_stop_info.k == current)
> > return 1;
> > try_to_freeze();
> > return 0;
> > }
>
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
>
> Perhaps we should introduce a new helper which does this.
Agreed.
Rafael
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
> On 03/02, Paul E. McKenney wrote:
> >
> > One way to embed try_to_freeze() into kthread_should_stop() might be
> > as follows:
> >
> > int kthread_should_stop(void)
> > {
> > if (kthread_stop_info.k == current)
> > return 1;
> > try_to_freeze();
> > return 0;
> > }
>
> I think this is dangerous. For example, worker_thread() will probably
> need some special actions after return from refrigerator. Also, a kernel
> thread may check kthread_should_stop() in the place where try_to_freeze()
> is not safe.
>
> Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
Is this what you had in mind?
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On 03/02, Paul E. McKenney wrote:
>
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
>
> int kthread_should_stop(void)
> {
> if (kthread_stop_info.k == current)
> return 1;
> try_to_freeze();
> return 0;
> }
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
Hi Paul,
> We certainly either need to embed try_to_freeze() into kthread_should_stop()
> or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
> components of this patch. ;-)
>
> One way to embed try_to_freeze() into kthread_should_stop() might be
> as follows:
>
> int kthread_should_stop(void)
> {
> if (kthread_stop_info.k == current)
> return 1;
> try_to_freeze();
> return 0;
> }
>
> Does this seem reasonable? It certainly would cut down some of the
> code required for freezing -- and reduce the potential for bugs.
Thats what I was thinking too :)
Anton
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Fri, Mar 02, 2007 at 12:27:30PM +0530, Gautham R Shenoy wrote:
> > From: Paul E. McKenney <[EMAIL PROTECTED]>
> >
> > Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze()
> > call as
> > required.
> >
> > Signed-off-by: Paul E. McKenney <[EMAIL PROTECTED]>
> > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> > Acked-by: Pavel Machek <[EMAIL PROTECTED]>
> > ---
> > kernel/rcutorture.c |3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-2.6.20-mm2/kernel/rcutorture.c
> > ===
> > --- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25
> > 12:07:15.0 +0100
> > +++ linux-2.6.20-mm2/kernel/rcutorture.c2007-02-25 12:49:23.0
> > +0100
> > @@ -46,6 +46,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Paul E. McKenney <[EMAIL PROTECTED]> and "
> > @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
> >
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
> > set_user_nice(current, 19);
> > - current->flags |= PF_NOFREEZE;
> >
> > do {
> > schedule_timeout_uninterruptible(1);
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > + try_to_freeze();
> > } while (!kthread_should_stop() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > while (!kthread_should_stop())
>
> Paul,
> Any reasons for not try_to_freeze()'ing the fakewriter and the reader
> threads?? (Ok, I admit, I haven't looked into the code for the reason
> which might be obvious.)
None that I know of -- though I do like Anton's idea of burying a
try_to_freeze() in kthread_should_stop(), which would get rid of all
try_to_freeze() calls in rcutorture, and many other places as well.
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Thu, Mar 01, 2007 at 08:54:25PM +0100, Rafael J. Wysocki wrote:
> On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
> >
> > Hi,
> >
> > > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > > try_to_freeze() call as required.
> >
> > ...
> >
> > > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > > }
> > > rcu_torture_current_version++;
> > > oldbatch = cur_ops->completed();
> > > + try_to_freeze();
> > > } while (!kthread_should_stop() && !fullstop);
> > > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > > while (!kthread_should_stop())
> >
> > I wonder if it makes sense to embed try_to_freeze() into the kthread
> > API somewhere. Short of that we should document the try_to_freeze()
> > requirement in the kthread documentation... Unfortunately I cant find
> > any kthread docs in Documentation/ :)
>
> Well, the patch is from Paul, so I think he'll be able to comment. :-)
We certainly either need to embed try_to_freeze() into kthread_should_stop()
or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
components of this patch. ;-)
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
Does this seem reasonable? It certainly would cut down some of the
code required for freezing -- and reduce the potential for bugs.
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Thu, Mar 01, 2007 at 08:54:25PM +0100, Rafael J. Wysocki wrote:
On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
Hi,
Remove PF_NOFREEZE from the rcutorture thread, adding a
try_to_freeze() call as required.
...
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
+ try_to_freeze();
} while (!kthread_should_stop() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
while (!kthread_should_stop())
I wonder if it makes sense to embed try_to_freeze() into the kthread
API somewhere. Short of that we should document the try_to_freeze()
requirement in the kthread documentation... Unfortunately I cant find
any kthread docs in Documentation/ :)
Well, the patch is from Paul, so I think he'll be able to comment. :-)
We certainly either need to embed try_to_freeze() into kthread_should_stop()
or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
components of this patch. ;-)
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
Does this seem reasonable? It certainly would cut down some of the
code required for freezing -- and reduce the potential for bugs.
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Fri, Mar 02, 2007 at 12:27:30PM +0530, Gautham R Shenoy wrote:
From: Paul E. McKenney [EMAIL PROTECTED]
Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze()
call as
required.
Signed-off-by: Paul E. McKenney [EMAIL PROTECTED]
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by: Pavel Machek [EMAIL PROTECTED]
---
kernel/rcutorture.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/rcutorture.c
===
--- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25
12:07:15.0 +0100
+++ linux-2.6.20-mm2/kernel/rcutorture.c2007-02-25 12:49:23.0
+0100
@@ -46,6 +46,7 @@
#include linux/byteorder/swabb.h
#include linux/stat.h
#include linux/srcu.h
+#include linux/freezer.h
MODULE_LICENSE(GPL);
MODULE_AUTHOR(Paul E. McKenney [EMAIL PROTECTED] and
@@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
VERBOSE_PRINTK_STRING(rcu_torture_writer task started);
set_user_nice(current, 19);
- current-flags |= PF_NOFREEZE;
do {
schedule_timeout_uninterruptible(1);
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
+ try_to_freeze();
} while (!kthread_should_stop() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
while (!kthread_should_stop())
Paul,
Any reasons for not try_to_freeze()'ing the fakewriter and the reader
threads?? (Ok, I admit, I haven't looked into the code for the reason
which might be obvious.)
None that I know of -- though I do like Anton's idea of burying a
try_to_freeze() in kthread_should_stop(), which would get rid of all
try_to_freeze() calls in rcutorture, and many other places as well.
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
Hi Paul,
We certainly either need to embed try_to_freeze() into kthread_should_stop()
or add back the rcu_torture_fakewriter(), and rcu_torture_reader()
components of this patch. ;-)
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
Does this seem reasonable? It certainly would cut down some of the
code required for freezing -- and reduce the potential for bugs.
Thats what I was thinking too :)
Anton
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Sat, Mar 03, 2007 at 02:33:37AM +0300, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Good point -- the return value from try_to_freeze() is lost if one uses
the above approach. About one third of the calls to try_to_freeze()
in 2.6.20 pay attention to the return value.
One approach would be to have a kthread_should_stop_nofreeze() for those
cases, and let the default be to try to freeze.
Is this what you had in mind?
Thanx, Paul
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Saturday, 3 March 2007 00:33, Oleg Nesterov wrote:
On 03/02, Paul E. McKenney wrote:
One way to embed try_to_freeze() into kthread_should_stop() might be
as follows:
int kthread_should_stop(void)
{
if (kthread_stop_info.k == current)
return 1;
try_to_freeze();
return 0;
}
I think this is dangerous. For example, worker_thread() will probably
need some special actions after return from refrigerator. Also, a kernel
thread may check kthread_should_stop() in the place where try_to_freeze()
is not safe.
Perhaps we should introduce a new helper which does this.
Agreed.
Rafael
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
> From: Paul E. McKenney <[EMAIL PROTECTED]>
>
> Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call
> as
> required.
>
> Signed-off-by: Paul E. McKenney <[EMAIL PROTECTED]>
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> Acked-by: Pavel Machek <[EMAIL PROTECTED]>
> ---
> kernel/rcutorture.c |3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.20-mm2/kernel/rcutorture.c
> ===
> --- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25 12:07:15.0
> +0100
> +++ linux-2.6.20-mm2/kernel/rcutorture.c 2007-02-25 12:49:23.0
> +0100
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Paul E. McKenney <[EMAIL PROTECTED]> and "
> @@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
>
> VERBOSE_PRINTK_STRING("rcu_torture_writer task started");
> set_user_nice(current, 19);
> - current->flags |= PF_NOFREEZE;
>
> do {
> schedule_timeout_uninterruptible(1);
> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> + try_to_freeze();
> } while (!kthread_should_stop() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> while (!kthread_should_stop())
Paul,
Any reasons for not try_to_freeze()'ing the fakewriter and the reader
threads?? (Ok, I admit, I haven't looked into the code for the reason
which might be obvious.)
thanks
gautham.
--
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Thursday, 1 March 2007 20:38, Anton Blanchard wrote:
>
> Hi,
>
> > Remove PF_NOFREEZE from the rcutorture thread, adding a
> > try_to_freeze() call as required.
>
> ...
>
> > @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> > }
> > rcu_torture_current_version++;
> > oldbatch = cur_ops->completed();
> > + try_to_freeze();
> > } while (!kthread_should_stop() && !fullstop);
> > VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> > while (!kthread_should_stop())
>
> I wonder if it makes sense to embed try_to_freeze() into the kthread
> API somewhere. Short of that we should document the try_to_freeze()
> requirement in the kthread documentation... Unfortunately I cant find
> any kthread docs in Documentation/ :)
Well, the patch is from Paul, so I think he'll be able to comment. :-)
Greetings,
Rafael
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
Hi,
> Remove PF_NOFREEZE from the rcutorture thread, adding a
> try_to_freeze() call as required.
...
> @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
> }
> rcu_torture_current_version++;
> oldbatch = cur_ops->completed();
> + try_to_freeze();
> } while (!kthread_should_stop() && !fullstop);
> VERBOSE_PRINTK_STRING("rcu_torture_writer task stopping");
> while (!kthread_should_stop())
I wonder if it makes sense to embed try_to_freeze() into the kthread
API somewhere. Short of that we should document the try_to_freeze()
requirement in the kthread documentation... Unfortunately I cant find
any kthread docs in Documentation/ :)
Anton
-
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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
Hi, Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as required. ... @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg) } rcu_torture_current_version++; oldbatch = cur_ops-completed(); + try_to_freeze(); } while (!kthread_should_stop() !fullstop); VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping); while (!kthread_should_stop()) I wonder if it makes sense to embed try_to_freeze() into the kthread API somewhere. Short of that we should document the try_to_freeze() requirement in the kthread documentation... Unfortunately I cant find any kthread docs in Documentation/ :) Anton - 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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
On Thursday, 1 March 2007 20:38, Anton Blanchard wrote: Hi, Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call as required. ... @@ -607,6 +607,7 @@ rcu_torture_writer(void *arg) } rcu_torture_current_version++; oldbatch = cur_ops-completed(); + try_to_freeze(); } while (!kthread_should_stop() !fullstop); VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping); while (!kthread_should_stop()) I wonder if it makes sense to embed try_to_freeze() into the kthread API somewhere. Short of that we should document the try_to_freeze() requirement in the kthread documentation... Unfortunately I cant find any kthread docs in Documentation/ :) Well, the patch is from Paul, so I think he'll be able to comment. :-) Greetings, Rafael - 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 3/7] Freezer: Remove PF_NOFREEZE from rcutorture thread
From: Paul E. McKenney [EMAIL PROTECTED]
Remove PF_NOFREEZE from the rcutorture thread, adding a try_to_freeze() call
as
required.
Signed-off-by: Paul E. McKenney [EMAIL PROTECTED]
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
Acked-by: Pavel Machek [EMAIL PROTECTED]
---
kernel/rcutorture.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-2.6.20-mm2/kernel/rcutorture.c
===
--- linux-2.6.20-mm2.orig/kernel/rcutorture.c 2007-02-25 12:07:15.0
+0100
+++ linux-2.6.20-mm2/kernel/rcutorture.c 2007-02-25 12:49:23.0
+0100
@@ -46,6 +46,7 @@
#include linux/byteorder/swabb.h
#include linux/stat.h
#include linux/srcu.h
+#include linux/freezer.h
MODULE_LICENSE(GPL);
MODULE_AUTHOR(Paul E. McKenney [EMAIL PROTECTED] and
@@ -585,7 +586,6 @@ rcu_torture_writer(void *arg)
VERBOSE_PRINTK_STRING(rcu_torture_writer task started);
set_user_nice(current, 19);
- current-flags |= PF_NOFREEZE;
do {
schedule_timeout_uninterruptible(1);
@@ -607,6 +607,7 @@ rcu_torture_writer(void *arg)
}
rcu_torture_current_version++;
oldbatch = cur_ops-completed();
+ try_to_freeze();
} while (!kthread_should_stop() !fullstop);
VERBOSE_PRINTK_STRING(rcu_torture_writer task stopping);
while (!kthread_should_stop())
Paul,
Any reasons for not try_to_freeze()'ing the fakewriter and the reader
threads?? (Ok, I admit, I haven't looked into the code for the reason
which might be obvious.)
thanks
gautham.
--
Gautham R Shenoy
Linux Technology Center
IBM India.
Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!
-
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/
