From: Nishanth Menon <nm@ti.com>
To: viresh kumar <viresh.kumar@linaro.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: cpufreq@vger.kernel.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org, ceh@ti.com,
Tobias Diedrich <ranma+xen@tdiedrich.de>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Dave Jones <davej@redhat.com>
Subject: Re: [PATCH] cpufreq: stats: Do not populate stats when policy->cur has no exact match
Date: Mon, 18 Nov 2013 09:08:09 -0600 [thread overview]
Message-ID: <528A2D59.3080001@ti.com> (raw)
In-Reply-To: <5287010F.9000508@linaro.org>
On 11/15/2013 11:22 PM, viresh kumar wrote:
> On Saturday 16 November 2013 06:40 AM, Rafael J. Wysocki wrote:
>> On Friday, November 15, 2013 06:20:43 PM Nishanth Menon wrote:
>
>>> So, instead of having a statistics information that never ever
>>> reflects valid data in the mentioned case and scratching our heads, we
>>> instead, refuse to populate any of the statistics entries and note in
>>> kernel log the error condition for developers to fix. The only useable
>
> s/useable/usable
>
>>> information are the available frequencies which is already available
>>> in other cpufreq sysfs entries.
>
>> I like this one. Any objections from anyone?
>
> Well nothing against the patch but I have other thoughts. There are platforms
> which might have no choice of fixing this issue as their bootloaders might be
> setting boot freq to any value outside of our freq table.
>
> And those might not have a chance to fix that in driver as well in case they are
> using something like cpufreq-cpu0..
>
> So, eventually this patch wouldn't do anything except giving a boot time error
> and not initializing any stats at all..
>
> Wouldn't it be better to create another frequency in all these tables, which
> will be an *Invalid* frequency.. With a value of -1 (i.e. largest value of an
> unsigned int) ??
>
> And so nobody will ever miss stats again, even if they are running on invalid
> frequencies. We will capture that information too:
> - we have moved from/to invalid frequency to/from a valid/invalid frequency this
> much times.
> - We have stayed at valid/invalid frequencies for this much time.
>
> I have a untested patch for this. If this looks okay, Nishant can you please try
> below patch? With some fixups from your side :)
http://pastebin.mozilla.org/3628975
I agree that it does show something, but would we not rather prefer to
stick with the entries available in freq_table than have to deal with
invalid frequencies that may be provided by the driver? for example:
how do we in stat know that there will only be one invalid frequency
request?
>
[..]
> (@Rafael: Finally I have moved to thunderbird, found a way out, so no more
> crappy attachments from me :))
>
there are some patch wrapping that thunderbird tends to do - I prefer
mutt that way, when I need to send inline patches :).
--
Regards,
Nishanth Menon
WARNING: multiple messages have this Message-ID (diff)
From: Nishanth Menon <nm@ti.com>
To: viresh kumar <viresh.kumar@linaro.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: <cpufreq@vger.kernel.org>, <linux-pm@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <ceh@ti.com>,
Tobias Diedrich <ranma+xen@tdiedrich.de>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Dave Jones <davej@redhat.com>
Subject: Re: [PATCH] cpufreq: stats: Do not populate stats when policy->cur has no exact match
Date: Mon, 18 Nov 2013 09:08:09 -0600 [thread overview]
Message-ID: <528A2D59.3080001@ti.com> (raw)
In-Reply-To: <5287010F.9000508@linaro.org>
On 11/15/2013 11:22 PM, viresh kumar wrote:
> On Saturday 16 November 2013 06:40 AM, Rafael J. Wysocki wrote:
>> On Friday, November 15, 2013 06:20:43 PM Nishanth Menon wrote:
>
>>> So, instead of having a statistics information that never ever
>>> reflects valid data in the mentioned case and scratching our heads, we
>>> instead, refuse to populate any of the statistics entries and note in
>>> kernel log the error condition for developers to fix. The only useable
>
> s/useable/usable
>
>>> information are the available frequencies which is already available
>>> in other cpufreq sysfs entries.
>
>> I like this one. Any objections from anyone?
>
> Well nothing against the patch but I have other thoughts. There are platforms
> which might have no choice of fixing this issue as their bootloaders might be
> setting boot freq to any value outside of our freq table.
>
> And those might not have a chance to fix that in driver as well in case they are
> using something like cpufreq-cpu0..
>
> So, eventually this patch wouldn't do anything except giving a boot time error
> and not initializing any stats at all..
>
> Wouldn't it be better to create another frequency in all these tables, which
> will be an *Invalid* frequency.. With a value of -1 (i.e. largest value of an
> unsigned int) ??
>
> And so nobody will ever miss stats again, even if they are running on invalid
> frequencies. We will capture that information too:
> - we have moved from/to invalid frequency to/from a valid/invalid frequency this
> much times.
> - We have stayed at valid/invalid frequencies for this much time.
>
> I have a untested patch for this. If this looks okay, Nishant can you please try
> below patch? With some fixups from your side :)
http://pastebin.mozilla.org/3628975
I agree that it does show something, but would we not rather prefer to
stick with the entries available in freq_table than have to deal with
invalid frequencies that may be provided by the driver? for example:
how do we in stat know that there will only be one invalid frequency
request?
>
[..]
> (@Rafael: Finally I have moved to thunderbird, found a way out, so no more
> crappy attachments from me :))
>
there are some patch wrapping that thunderbird tends to do - I prefer
mutt that way, when I need to send inline patches :).
--
Regards,
Nishanth Menon
next prev parent reply other threads:[~2013-11-18 15:08 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-16 0:20 [PATCH] cpufreq: stats: Do not populate stats when policy->cur has no exact match Nishanth Menon
2013-11-16 0:20 ` Nishanth Menon
2013-11-16 1:10 ` Rafael J. Wysocki
2013-11-16 5:22 ` viresh kumar
2013-11-18 15:08 ` Nishanth Menon [this message]
2013-11-18 15:08 ` Nishanth Menon
2013-11-18 15:22 ` Viresh Kumar
2013-11-18 15:34 ` Nishanth Menon
2013-11-18 15:43 ` Viresh Kumar
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=528A2D59.3080001@ti.com \
--to=nm@ti.com \
--cc=ceh@ti.com \
--cc=cpufreq@vger.kernel.org \
--cc=davej@redhat.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=ranma+xen@tdiedrich.de \
--cc=rjw@rjwysocki.net \
--cc=viresh.kumar@linaro.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.