linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Brian Norris <computersforpeace@gmail.com>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Len Brown <len.brown@intel.com>,
	Chirantan Ekbote <chirantan@chromium.org>
Subject: Re: [PATCH] PM / sleep: add configurable delay for pm_test
Date: Sun, 22 Feb 2015 00:24:29 +0100	[thread overview]
Message-ID: <20150221232429.GA3519@amd> (raw)
In-Reply-To: <CAGVrzcYSHem65m6ChSF83qu5AF_fQVZj=qdV+vNNtSKZM1YLXg@mail.gmail.com>

On Sat 2015-02-21 14:56:01, Florian Fainelli wrote:
> 2015-02-21 12:32 GMT-08:00 Pavel Machek <pavel@ucw.cz>:
> >
> >
> >
> >> Considering that Brian's change are enclosed within a CONFIG_PM_DEBUG
> >> ifdef, can we really use the code bloat as a technical argument here?
> >
> > Yes.
> 
> Help me understand a few things here:

What you are missing is that we try to keep _sources_ small and
readable, too.

> - if we need to turn on PM_DEBUG all the time, including mainline
> distributions, does that mean that:
>            - portions of code existing only in PM_DEBUG should be
> moved out of it because it is actually useful outside of debug option?
>            - CONFIG_PM itself is not self sufficient and there are
> still problems that require PM_DEBUG to be turned on?
>            - should there be a second level debug option (e.g:
> CONFIG_PM_DEBUG_ADV) which gates specific control knobs like PM delay?
> 
> The current 5 seconds delay is completely arbitrary and goes against
> the principle of not enforcing a policy, having this configurable
> brings this back in the mechanism principle.

5 second delay is arbitrary, and slightly ugly debugging hack, that is
acceptable because it is useful, small and unobtrusive. The second you
add 40 lines of sysfs glue.. it is no longer small and unobtrusive,
and no longer acceptable.

And yes, distros probably ship with PM_DEBUG on, and no, adding
another config option would not make the big & ugly hack more
acceptable.

OTOH if you added a hook to kprobes that alowed easy binary patching
of the value "5" while not adding 40 lines of overhead, that might be
neccessary.

Actually, you can probably pull the address from System.map and then
just write to it using /dev/mem, no? Just arrange for "5" to be in
variable that is in System.map.

And you also want to check the kernel parameters, they are modifiable
in runtime in at least same cases.

But we are not adding 40 lines of uglyness. Clear?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

  reply	other threads:[~2015-02-21 23:24 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 23:55 [PATCH] PM / sleep: add configurable delay for pm_test Brian Norris
     [not found] ` <20140904071412.GA29832@amd>
2014-09-04 17:54   ` Brian Norris
2014-09-05  2:11     ` Chirantan Ekbote
2014-12-13  2:55 ` Brian Norris
2014-12-13  8:31   ` Pavel Machek
2014-12-16 23:58     ` Brian Norris
2014-12-17  8:09       ` Pavel Machek
2014-12-17  9:10         ` Brian Norris
2014-12-17  9:22           ` Pavel Machek
2015-02-21 20:25             ` Florian Fainelli
2015-02-21 20:32               ` Pavel Machek
2015-02-21 22:56                 ` Florian Fainelli
2015-02-21 23:24                   ` Pavel Machek [this message]
2015-02-22  8:23                     ` Brian Norris
2015-02-22  8:26 ` [PATCH v2] " Brian Norris
2015-02-22 12:27   ` Pavel Machek
2015-02-22 15:17     ` Kevin Cernekee
2015-02-22 19:25   ` Florian Fainelli
2015-02-23  5:16   ` [PATCH v3] " Brian Norris

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=20150221232429.GA3519@amd \
    --to=pavel@ucw.cz \
    --cc=chirantan@chromium.org \
    --cc=computersforpeace@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /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).