linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: l.majewski@samsung.com (Lukasz Majewski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] cpufreq: tests: Providing cpufreq regression test
Date: Wed, 23 Jul 2014 12:10:42 +0200	[thread overview]
Message-ID: <20140723121042.5b6f49e3@amdc2363> (raw)
In-Reply-To: <CAKohpok++gPcNk9-E4MsFdNBy8qL7oLYvatk1EvcW1OHwroZXQ@mail.gmail.com>

Hi Viresh,

> On 23 July 2014 13:08, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > Do you want to say that we have enough tests and we don't need
> > more ?
> 
> No. We don't have any tests at all :)

Then we should encourage as many developers as possible to share their
private tests with us.

> 
> > I always thought that we shall have as much regression tests as
> > possible.
> 
> Yeah, tests are welcomed but the question is where should they get
> added. Don't know if its common to add tests directly to kernel.

There was a similar discussion with device tree and finally it was
included in the mainline repository.

> 
> And also if the test is really good, not discouraging your work.
> 
> >> On 21 July 2014 12:32, Lukasz Majewski <l.majewski@samsung.com>
> >> wrote:
> >> > This commit adds first regression test "cpufreq_freq_test.sh" for
> >> > the cpufreq subsystem.
> >>
> >> That's not enough, Tell us why we should continue reading this
> >> mail..
> >
> > Hmm... If "regression" and "test" don't catch the attention of a
> > diligent maintainer, then I cannot do much more to encourage him to
> > read the whole e-mail :-)
> 
> What I meant to say was, your subject and body must be good enough
> to answer most of the things. You don't have to tell much about the
> implementation but other things should be pretty clear from logs.
> 
> Your current logs are quite short for something that's not a normal
> practice.

It is hard for me to agree on this issue.

> 
> > I can imagine that maintainers are very busy, therefore I've
> > prepared README file with detailed description of the script
> > operation.
> 
> Yeah, a README is welcomed and would be useful for users as well..
> 
> >> I couldn't make out the purpose of this test and why we need it.
> >> How do we ensure that "cpufreq attributes exported by sysfs are
> >> exposing correct values"?
> >
> > First of all the cpufreq attributes are part of the subsystem API.
> > There are systems which actually depend on them, so we would be
> > better off to test if they work as intended.
> >
> > Secondly, the test takes those values and then with use of other
> > attribute enforce the value, which is then read via cat'ing
> > cpufreq_cur_freq. If any of the attributes is wrong then we will
> > spot the error immediately.
> 
> Shouldn't you use userspace governor then instead of performance?

Performance assures that we will have the right frequency set.

However, there can be a similar patch to use userspace governor and
various load to fail if ondemand's frequency flipping is detected.

> And then we don't need the gzip stuff at all. We can just set it to
> the right freq and get current freq to see if it matches?

Sometimes "interresting" things show up when you have 100% CPU load and
you try to switch frequency.
In my opinion usage of gzip makes the test more difficult to pass.

> 
> And now that we are starting to get tests added into the kernel (will
> still wait to see what Rafael has to advice),

Ok. Lets wait for Rafael's opinion.

> we better think of the
> way these are going to get added. Probably a single script with
> parameters like what to test?

It is one possible solution, where another one is to run the all scripts
in the directory.

I'm curious about Rafael's opinion.

> 
> >> And actually what do we mean by this statement even? What kind of
> >> errors can be there in exposing these values.
> >
> > Errors with cpufreq and CCF cooperation - especially when some
> > parts of cpufreq code uses direct write to MUX, DIV or PLL SoC
> > registers.
> >
> > Also, one can check if permutations of changing all available
> > frequencies are working properly.
> 
> Yeah, that would be fine. Probably need to think more about scripts
> name.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

  reply	other threads:[~2014-07-23 10:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-18 10:23 [PATCH] cpufreq: tests: Providing cpufreq regression test Lukasz Majewski
2014-07-18 11:28 ` Sachin Kamat
2014-07-18 11:59   ` Lukasz Majewski
2014-07-22  4:13     ` Sachin Kamat
2014-07-18 11:42 ` Rafael J. Wysocki
2014-07-18 12:00   ` Lukasz Majewski
2014-07-21  7:02 ` [PATCH v2] " Lukasz Majewski
2014-07-23  5:06   ` Viresh Kumar
2014-07-23  7:38     ` Lukasz Majewski
2014-07-23  8:49       ` Viresh Kumar
2014-07-23 10:10         ` Lukasz Majewski [this message]
2014-07-23 10:48           ` Viresh Kumar
2014-07-24 10:05           ` Javi Merino
2014-07-23 11:58         ` Rafael J. Wysocki
2014-07-23 15:02         ` Andrew Lunn
2014-07-23 23:58   ` Rafael J. Wysocki
2014-07-24  7:04     ` Lukasz Majewski
2014-08-06 22:50       ` Rafael J. Wysocki

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=20140723121042.5b6f49e3@amdc2363 \
    --to=l.majewski@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).