linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Boyd <swboyd@chromium.org>
To: Tri Vo <trong@android.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Hridya Valsaraju <hridya@google.com>,
	Sandeep Patil <sspatil@google.com>,
	Kalesh Singh <kaleshsingh@google.com>,
	Ravi Chandra Sadineni <ravisadineni@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux PM <linux-pm@vger.kernel.org>,
	"Cc: Android Kernel" <kernel-team@android.com>
Subject: Re: [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs
Date: Thu, 01 Aug 2019 15:11:57 -0700	[thread overview]
Message-ID: <5d4363ae.1c69fb81.b621e.65ed@mx.google.com> (raw)
In-Reply-To: <CANA+-vCt3QJDykzbZWBDZyaiaMiz_SOJ+Htv7+G0czjL07MjmQ@mail.gmail.com>

Quoting Tri Vo (2019-08-01 14:44:52)
> On Thu, Aug 1, 2019 at 1:23 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> >
> > I don't find it awkward or difficult. Just know what the name of the
> > /sys/class/wakeup/ path is and then extract the name from there if it
> > doesn't match wakeupN, otherwise read the 'device' symlink and run it
> > through basename.
> 
> The concern was that having both "id" and "name" around might be
> confusing. I don't think that making the presence of "name"
> conditional helps here. And we have to maintain additional logic in
> both kernel and userspace to support this.
> 
> Also, say, userspace grabs a wakelock named "wakeup0". In the current
> patch, this results in a name collision and an error. Even assuming
> that userspace doesn't have ill intent, it still needs to be aware of
> "wakeupN" naming pattern to avoid this error condition.
> 
> All wakeup sources in the /sys/class/wakeup/ are in the same namespace
> regardless of where they originate from, i.e. we have to either (1)
> inspect the name of a wakeup source and make sure it's unique before
> using it as a directory name OR (2) generate the directory name on
> behalf of whomever is registering a wakeup source, which I think is a
> much simpler solution.

Ok. If the device name is going to be something generic like 'wakeupN',
then we need to make sure that the wakeup source name is unique.
Otherwise, I'm not able to see how userspace will differentiate between
two of the same named wakelocks. Before this patch the wakeup source
name looks to have been used for debugging, but now it's being used
programmatically to let userspace act upon it somehow. Maybe it's for
debug still, but I could see how userspace may want to hunt down the
wakelock that's created in userspace and penalize or kill the task
that's waking up the device.

I see that wakelock_lookup_add() already checks the list of wakelock
wakeup sources, but I don't see how I can't create an "alarmtimer"
wakelock again, but this time for userspace, by writing into
/sys/power/wake_lock.

What happens with namespaces here BTW? Can a wakelock be made in one
namespace and that is the same name as another wakelock in a different
namespace? Right now it doesn't look possible because of the global name
matching, but it probably makes sense to support this? Maybe we just
shouldn't make anything in sysfs for wake sources that can be any random
name created from the wakelock path right now. I don't see how it can be
traced back to the process that created it in any reasonable way.


  parent reply	other threads:[~2019-08-01 22:12 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-31 21:55 [PATCH v6] PM / wakeup: show wakeup sources stats in sysfs Tri Vo
2019-07-31 21:59 ` Stephen Boyd
2019-07-31 22:16   ` Tri Vo
2019-07-31 22:17   ` Rafael J. Wysocki
2019-07-31 22:31     ` Tri Vo
2019-07-31 22:42       ` Rafael J. Wysocki
2019-07-31 22:58         ` Tri Vo
2019-07-31 23:10           ` Rafael J. Wysocki
2019-07-31 23:27             ` Tri Vo
2019-07-31 23:45             ` Stephen Boyd
2019-08-01  0:45               ` Stephen Boyd
2019-08-01  8:09                 ` Rafael J. Wysocki
2019-08-01 15:31                   ` Stephen Boyd
2019-08-01 17:21                     ` Rafael J. Wysocki
2019-08-01 19:25                       ` Stephen Boyd
2019-08-01 19:36                         ` Greg Kroah-Hartman
2019-08-01 19:50               ` Tri Vo
2019-08-01 20:22                 ` Stephen Boyd
2019-08-01 21:44                   ` Tri Vo
2019-08-01 22:10                     ` Rafael J. Wysocki
2019-08-03 22:40                       ` Tri Vo
2019-08-05  8:32                         ` Rafael J. Wysocki
2019-08-01 22:11                     ` Stephen Boyd [this message]
2019-08-01 22:44                       ` Tri Vo
2019-08-01 23:37                         ` Stephen Boyd
2019-08-01 22:46                       ` Rafael J. Wysocki
2019-08-01 23:40                         ` Stephen Boyd
2019-07-31 23:00         ` 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=5d4363ae.1c69fb81.b621e.65ed@mx.google.com \
    --to=swboyd@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hridya@google.com \
    --cc=kaleshsingh@google.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=ravisadineni@chromium.org \
    --cc=rjw@rjwysocki.net \
    --cc=sspatil@google.com \
    --cc=trong@android.com \
    --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 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).