linux-kernel - Re: [PATCH] PM / sleep: add configurable delay for pm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20141216235813.GI9759@ld-irv-0074>
Date:	Tue, 16 Dec 2014 15:58:13 -0800
From:	Brian Norris <computersforpeace@...il.com>
To:	Pavel Machek <pavel@....cz>
Cc:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	linux-pm@...r.kernel.org, Len Brown <len.brown@...el.com>,
	Chirantan Ekbote <chirantan@...omium.org>
Subject: Re: [PATCH] PM / sleep: add configurable delay for pm_test

On Sat, Dec 13, 2014 at 09:31:23AM +0100, Pavel Machek wrote:
> On Fri 2014-12-12 18:55:30, Brian Norris wrote:
> > On Wed, Sep 03, 2014 at 04:55:35PM -0700, Brian Norris wrote:
> > > When CONFIG_PM_DEBUG=y, we provide a sysfs file (/sys/power/pm_test) for
> > > selecting one of a few suspend test modes, where rather than entering a
> > > full suspend state, the kernel will perform some subset of suspend
> > > steps, wait 5 seconds, and then resume back to normal operation.
> > > 
> > > This mode is useful for (among other things) observing the state of the
> > > system just before entering a sleep mode, for debugging or analysis
> > > purposes. However, a constant 5 second wait is not sufficient for some
> > > sorts of analysis; for example, on an SoC, one might want to use
> > > external tools to probe the power states of various on-chip controllers
> > > or clocks.
> > > 
> > > This patch adds a companion sysfs file (/sys/power/pm_test_delay) that
> > > allows user-space to configure how long the system waits in this test
> > > state before resuming. It also updates the PM debugging documentation to
> > > mention the new file.
> > > 
> > > Signed-off-by: Brian Norris <computersforpeace@...il.com>
> > 
> > What do you think about this patch? It seems there is at least one other
> > developer who is independently interested in this.
> 
> 40 lines of code, and new sysfs interface for use by someone who puts
> the probes on board, anyway... (so should be able to add the single
> mdelay himself).

I heard your complaint the first time:

  https://lkml.org/lkml/2014/9/4/63

And I responded to it already:

  https://lkml.org/lkml/2014/9/4/494

You did not respond, but Chirantan spoke up saying he wanted such a
patch too. He came up with a very similar solution independently:

  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/15bccc2c63c3475ef61d3187c73ccf1d80b18c7e

But since you've decided to make the same comment again, I will detail
more of the reasons why I think your suggestion ("go add the mdelay
yourself") is off-base.

 1. This is behind a debug config option (CONFIG_PM_DEBUG). So what's
 the problem with improving its usefulness? Non-debug users can easily
 compile it out if they're worried about 40 lines.

 2. The current debug code encodes a particular policy (which kernels
 generally should not). Is it better if I submit a patch that changes
 the current magic delay to 60000 milliseconds? What about 1334
 milliseconds?

 3. To continue your argument: why would I ever try to patch the
 upstream kernel, if I'm perfectly capable of doing this myself?

 4. How does putting probes on a board suddenly qualify someone for
 patching and rebuilding their kernel? I noted that I have *users* who
 want to do this. Hence, I'm patching a *user* interface.

Brian
--
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/