Re: [PATCH] make refrigerator cold

Re: [PATCH] make refrigerator cold

[Rafael J. Wysocki]

On Tuesday 22 September 2009, Stephen Hemminger wrote:
> By marking it cold, then the code path in kernel thread 
> usage of try_to_freeze() that is normally used be
> selected.

The sentence above isn't very clear IMO, could you please rephrase?

> Don't think it matters that much for performance but the
> concordance of this patch struck me as humorous.

Well, yeah.

> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/include/linux/freezer.h	2009-09-21 20:34:17.579233461 -0700
> +++ b/include/linux/freezer.h	2009-09-21 20:35:25.724322028 -0700
> @@ -47,7 +47,7 @@ static inline bool should_send_signal(st
>  /* Takes and releases task alloc lock using task_lock() */
>  extern int thaw_process(struct task_struct *p);
>  
> -extern void refrigerator(void);
> +extern void refrigerator(void) __cold;
>  extern int freeze_processes(void);
>  extern void thaw_processes(void);

Re: [PATCH] make refrigerator cold

[Stephen Hemminger]

On Tue, 22 Sep 2009 21:54:09 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Tuesday 22 September 2009, Stephen Hemminger wrote:
> > By marking it cold, then the code path in kernel thread 
> > usage of try_to_freeze() that is normally used be
> > selected.
> 

In the code for try_to_freeze(), for optimization, it might
help to tell the compiler to not favor the code path where
the refigrator is being called.

Another way to do the same thing would be to do.
   if (unlikely(freezing(current))) {
	refrigerator();
	return 1;
   } else
	return 0;

or build unlikely into the freezing function (see need_resched).

I saw this by trying to minimize the number of intstructions
in pktgen which is a special case.

Re: [PATCH] make refrigerator cold

[Rafael J. Wysocki]

On Tuesday 22 September 2009, Stephen Hemminger wrote:
> On Tue, 22 Sep 2009 21:54:09 +0200
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > On Tuesday 22 September 2009, Stephen Hemminger wrote:
> > > By marking it cold, then the code path in kernel thread 
> > > usage of try_to_freeze() that is normally used be
> > > selected.
> > 
> 
> In the code for try_to_freeze(), for optimization, it might
> help to tell the compiler to not favor the code path where
> the refigrator is being called.
> 
> Another way to do the same thing would be to do.
>    if (unlikely(freezing(current))) {
> 	refrigerator();
> 	return 1;
>    } else
> 	return 0;
> 
> or build unlikely into the freezing function (see need_resched).
> 
> I saw this by trying to minimize the number of intstructions
> in pktgen which is a special case.

OK, thanks.

Will it be fine with you if I add the patch to the suspend-2.6 tree with the
above information in the changelog?

Rafael

Re: [PATCH] make refrigerator cold

[Stephen Hemminger]

On Wed, 23 Sep 2009 01:13:20 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Tuesday 22 September 2009, Stephen Hemminger wrote:
> > On Tue, 22 Sep 2009 21:54:09 +0200
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > On Tuesday 22 September 2009, Stephen Hemminger wrote:
> > > > By marking it cold, then the code path in kernel thread 
> > > > usage of try_to_freeze() that is normally used be
> > > > selected.
> > > 
> > 
> > In the code for try_to_freeze(), for optimization, it might
> > help to tell the compiler to not favor the code path where
> > the refigrator is being called.
> > 
> > Another way to do the same thing would be to do.
> >    if (unlikely(freezing(current))) {
> > 	refrigerator();
> > 	return 1;
> >    } else
> > 	return 0;
> > 
> > or build unlikely into the freezing function (see need_resched).
> > 
> > I saw this by trying to minimize the number of intstructions
> > in pktgen which is a special case.
> 
> OK, thanks.
> 
> Will it be fine with you if I add the patch to the suspend-2.6 tree with the
> above information in the changelog?
> 
> Rafael

Sure, any of the possibilities works, you choose.