linux-kernel - Re: [PATCH v2] PM / wakeup: show wakeup sources stats in sysfs

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: <3451189.hAX5mKGt1u@kreacher>
Date:   Thu, 04 Jul 2019 12:31:24 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Tri Vo <trong@...roid.com>, viresh.kumar@...aro.org,
        rafael@...nel.org, hridya@...gle.com, sspatil@...gle.com,
        kaleshsingh@...gle.com, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v2] PM / wakeup: show wakeup sources stats in sysfs

On Friday, June 28, 2019 5:10:40 PM CEST Greg KH wrote:
> On Thu, Jun 27, 2019 at 03:53:35PM -0700, Tri Vo wrote:
> > Userspace can use wakeup_sources debugfs node to plot history of suspend
> > blocking wakeup sources over device's boot cycle. This information can
> > then be used (1) for power-specific bug reporting and (2) towards
> > attributing battery consumption to specific processes over a period of
> > time.
> > 
> > However, debugfs doesn't have stable ABI. For this reason, expose wakeup
> > sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
> > 
> > Embedding a struct kobject into struct wakeup_source changes lifetime
> > requirements on the latter. To that end, change deallocation of struct
> > wakeup_source using kfree to kobject_put().
> > 
> > Change struct wakelock's wakeup_source member to a pointer to decouple
> > lifetimes of struct wakelock and struct wakeup_source for above reason.
> > 
> > Introduce CONFIG_PM_SLEEP_STATS that enables/disables showing wakeup
> > source statistics in sysfs.
> > 
> > Signed-off-by: Tri Vo <trong@...roid.com>
> 
> Ok, this looks much better, but I don't like the use of a "raw" kobject
> here.  It is much simpler, and less code, to use 'struct device'
> instead.
> 
> As proof, I reworked the patch to do just that, and it saves over 50
> lines of .c code, which is always nice :)

Thanks for taking the time to do that!

> Attached below is the reworked code, along with the updated
> documentation file.  It creates devices in a virtual class, and you can
> easily iterate over them all by looking in /sys/class/wakeup/.

That actually is nice - no need to add anything under /sys/power/.

> Note, I'm note quite sure you need all of the changes you made in
> kernel/power/wakelock.c when you make the structure contain a pointer to
> the wakeup source and not the structure itself, but I just went with it
> and got it all to build properly.

I'm not really sure about it either.

> Also note, I've not actually tested this at all, only built it, so I
> _strongly_ suggest that you test this to make sure it really works :)
> 
> What do you think?

I agree with the direction. :-)

Cheers!