From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Borislav Petkov <bp@alien8.de>,
linux-kernel@vger.kernel.org, davej@redhat.com,
tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
x86@kernel.org, cpufreq@vger.kernel.org, andre.prz
Subject: Re: [PATCH 1/2] powernow-k8: Don't notify of successful transition if we failed.
Date: Wed, 15 Jun 2011 18:26:08 -0400 [thread overview]
Message-ID: <20110615222608.GA14031@dumpdata.com> (raw)
In-Reply-To: <20110615221636.GB9725@liondog.tnic>
> > @@ -1112,6 +1114,9 @@ static int transition_frequency_pstate(struct powernow_k8_data *data,
> > }
> >
> > res = transition_pstate(data, pstate);
> > + if (res)
> > + return res;
>
> That's wrong because transition_pstate() returns 0 unconditionally
> (at least it does so on 3.0-rc3). But this change accidentally fixes
> a different bug because res is used uninitialized, containing stack
> garbage otherwise.
>
> A proper fix should be to check against data->max_hw_pstate and
> check whether the entry is not CPUFREQ_ENTRY_INVALID (look at
> fill_powernow_table_pstate() for example).
Aha! I can respin a patch for that tomorrow.
>
> I'm guessing this oops happens when powernow-k8 is loaded in the guest
> and that the actual power management is done in the hypervisor?
Yes.
WARNING: multiple messages have this Message-ID (diff)
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Borislav Petkov <bp@alien8.de>,
linux-kernel@vger.kernel.org, davej@redhat.com,
tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com,
x86@kernel.org, cpufreq@vger.kernel.org, andre.przywara@amd.com,
Mark.Langsdorf@amd.com
Subject: Re: [PATCH 1/2] [CPUFREQ] powernow-k8: Don't notify of successful transition if we failed.
Date: Wed, 15 Jun 2011 18:26:08 -0400 [thread overview]
Message-ID: <20110615222608.GA14031@dumpdata.com> (raw)
In-Reply-To: <20110615221636.GB9725@liondog.tnic>
> > @@ -1112,6 +1114,9 @@ static int transition_frequency_pstate(struct powernow_k8_data *data,
> > }
> >
> > res = transition_pstate(data, pstate);
> > + if (res)
> > + return res;
>
> That's wrong because transition_pstate() returns 0 unconditionally
> (at least it does so on 3.0-rc3). But this change accidentally fixes
> a different bug because res is used uninitialized, containing stack
> garbage otherwise.
>
> A proper fix should be to check against data->max_hw_pstate and
> check whether the entry is not CPUFREQ_ENTRY_INVALID (look at
> fill_powernow_table_pstate() for example).
Aha! I can respin a patch for that tomorrow.
>
> I'm guessing this oops happens when powernow-k8 is loaded in the guest
> and that the actual power management is done in the hypervisor?
Yes.
next prev parent reply other threads:[~2011-06-15 22:26 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-15 19:01 [PATCH] cpufreq bug fixes (stable/cpufreq.bugfixes) for 3.0 (or 3.1) Konrad Rzeszutek Wilk
2011-06-15 19:01 ` [PATCH 1/2] [CPUFREQ] powernow-k8: Don't notify of successful transition if we failed Konrad Rzeszutek Wilk
2011-06-15 22:16 ` [PATCH 1/2] " Borislav Petkov
2011-06-15 22:16 ` [PATCH 1/2] [CPUFREQ] " Borislav Petkov
2011-06-15 22:26 ` Konrad Rzeszutek Wilk [this message]
2011-06-15 22:26 ` Konrad Rzeszutek Wilk
2011-06-16 14:24 ` [PATCH 1/2] " Konrad Rzeszutek Wilk
2011-06-16 14:24 ` [PATCH 1/2] [CPUFREQ] " Konrad Rzeszutek Wilk
2011-06-16 15:45 ` [PATCH 1/2] " Borislav Petkov
2011-06-16 15:45 ` [PATCH 1/2] [CPUFREQ] " Borislav Petkov
2011-06-15 19:02 ` [PATCH 2/2] : Don't set stat->last_index to -1 if the pol->cur has incorrect value Konrad Rzeszutek Wilk
2011-06-15 19:02 ` [PATCH 2/2] [CPUFREQ]: " Konrad Rzeszutek Wilk
2011-06-15 19:26 ` [PATCH] cpufreq bug fixes (stable/cpufreq.bugfixes) for 3.0 (or 3.1) Dave Jones
2011-06-15 21:13 ` Konrad Rzeszutek Wilk
2011-06-16 14:28 ` [PATCH] powernow-k8: Don't try to transition if the pstate is incorrect or there is no freq for it Konrad Rzeszutek Wilk
2011-06-16 14:28 ` [PATCH] [CPUFREQ] " Konrad Rzeszutek Wilk
2011-06-16 17:06 ` [PATCH] " Borislav Petkov
2011-06-16 17:06 ` [PATCH] [CPUFREQ] " Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110615222608.GA14031@dumpdata.com \
--to=konrad.wilk@oracle.com \
--cc=bp@alien8.de \
--cc=cpufreq@vger.kernel.org \
--cc=davej@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.