linux-kernel - Re: [PATH V2 4/5] Package Level Thermal Control and Power Limit Notification: power limit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20100728231508.GB13469@fenghua-desk.sc.intel.com>
Date:	Wed, 28 Jul 2010 16:15:08 -0700
From:	Fenghua Yu <fenghua.yu@...el.com>
To:	Guenter Roeck <guenter.roeck@...csson.com>
Cc:	"Yu, Fenghua" <fenghua.yu@...el.com>, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	H Peter Anvin <hpa@...or.com>, Len Brown <lenb@...nel.org>,
	Chen Gong <gong.chen@...ux.jf.intel.com>,
	Jean Delvare <khali@...ux-fr.org>,
	"Wan, Huaxu" <huaxu.wan@...el.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	lm-sensors <lm-sensors@...sensors.org>
Subject: Re: [PATH V2 4/5] Package Level Thermal Control and Power Limit
	Notification: power limit

On Wed, Jul 28, 2010 at 02:17:40PM -0700, Guenter Roeck wrote:
> On Wed, 2010-07-28 at 15:00 -0400, Fenghua Yu wrote:
> > From: Fenghua Yu <fenghua.yu@...el.com>
> >
> > -static int therm_throt_process(bool is_throttled, int level)
> > +static int therm_throt_process(bool new_event, int event, int level)
> >  {
> >         struct _thermal_state *state;
> > -       unsigned int this_cpu;
> > -       bool was_throttled;
> > +       unsigned int this_cpu = smp_processor_id();
> > +       bool old_event;
> >         u64 now;
> > +       struct thermal_state *pstate = &per_cpu(thermal_state, this_cpu);
> > +
> > +       if (!new_event)
> > +               return 0;
> >
> As a result of this check and return, events will never be reset.

With the check, throttling returning to normal will not be reflected (i.e. only
throttling event is shown). I'll remove this check.
> 
> > -       this_cpu = smp_processor_id();
> >         now = get_jiffies_64();
> > -       if (level == CORE_LEVEL)
> > -               state = &per_cpu(thermal_state, this_cpu).core;
> > -       else
> > -               state = &per_cpu(thermal_state, this_cpu).package;
> > +       if (level == CORE_LEVEL) {
> > +               if (event == THERMAL_THROTTLING_EVENT)
> > +                       state = &pstate->core_throttle;
> > +               else if (event == POWER_LIMIT_EVENT)
> > +                       state = &pstate->core_power_limit;
> > +               else
> > +                        return 0;
> > +       } else if (level == PACKAGE_LEVEL) {
> > +               if (event == THERMAL_THROTTLING_EVENT)
> > +                       state = &pstate->package_throttle;
> > +               else if (event == POWER_LIMIT_EVENT)
> > +                       state = &pstate->package_power_limit;
> > +               else
> > +                       return 0;
> > +       } else
> > +               return 0;
> >
> > -       was_throttled = state->is_throttled;
> > -       state->is_throttled = is_throttled;
> > +       old_event = state->new_event;
> > +       state->new_event = new_event;
> >
> > -       if (is_throttled)
> > -               state->throttle_count++;
> > +       if (new_event)
> > +               state->count++;
> >
> >         if (time_before64(now, state->next_check) &&
> > -                       state->throttle_count != state->last_throttle_count)
> > +                       state->count != state->last_count)
> >                 return 0;
> >
> >         state->next_check = now + CHECK_INTERVAL;
> > -       state->last_throttle_count = state->throttle_count;
> > +       state->last_count = state->count;
> >
> >         /* if we just entered the thermal event */
> > -       if (is_throttled) {
> > -               printk(KERN_CRIT "CPU%d: %s temperature above threshold, cpu clock throttled (total events = %lu)\n",
> > -                     this_cpu,
> > -                     level == CORE_LEVEL ? "Core" : "Package",
> > -                     state->throttle_count);
> > +       if (new_event) {
> > +               if (event == THERMAL_THROTTLING_EVENT)
> > +                       printk(KERN_CRIT "CPU%d: %s temperature above threshold, cpu clock throttled (total events = %lu)\n",
> > +                               this_cpu,
> > +                               level == CORE_LEVEL ? "Core" : "Package",
> > +                               state->count);
> > +               else
> > +                       printk(KERN_CRIT "CPU%d: %s power limit notification (total events = %lu)\n",
> > +                               this_cpu,
> > +                               level == CORE_LEVEL ? "Core" : "Package",
> > +                               state->count);
> >
> >                 add_taint(TAINT_MACHINE_CHECK);
> >                 return 1;
> >         }
> 
> Since new_event is always true, code below will never be executed.
> That doesn't make sense to me.
After removing the previous new_event check, this code should be fine.

Thanks.

-Fenghua
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/