linux-kernel - Re: [PATCH] drivers: base: add support to skip power management in device/driver model

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: <CAPDyKFpkiaP0J_LJxP6tRUOyz3QKtDgA+PuhL-9vew7-RfKyNA@mail.gmail.com>
Date:   Thu, 7 Feb 2019 15:29:07 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Jisheng Zhang <jszhang@...vell.com>,
        Steve Longerbeam <slongerbeam@...il.com>,
        Eugeniu Rosca <erosca@...adit-jv.com>,
        Joshua Frkuska <joshua_frkuska@...tor.com>,
        Eugeniu Rosca <roscaeugeniu@...il.com>
Subject: Re: [PATCH] drivers: base: add support to skip power management in
 device/driver model

On Thu, 7 Feb 2019 at 11:36, Sudeep Holla <sudeep.holla@....com> wrote:
>
> On Thu, Feb 07, 2019 at 10:53:56AM +0100, Ulf Hansson wrote:
> > On Wed, 6 Feb 2019 at 16:09, Sudeep Holla <sudeep.holla@....com> wrote:
> > >
> > > All device objects in the driver model contain fields that control the
> > > handling of various power management activities. However, it's not
> > > always useful. There are few instances where pseudo devices are added
> > > to the model just to take advantage of many other features like
> > > kobjects, udev events, and so on. One such example is cpu devices and
> > > their caches.
> > >
> > > The sysfs for the cpu caches are managed by adding devices with cpu
> > > as the parent in cpu_device_create() when secondary cpu is brought
> > > online. Generally when the secondary CPUs are hotplugged back in as part
> > > of resume from suspend-to-ram, we call cpu_device_create() from the cpu
> > > hotplug state machine while the cpu device associated with that CPU is
> > > not yet ready to be resumed as the device_resume() call happens bit
> > > later. It's not really needed to set the flag is_prepared for cpu
> > > devices as they are mostly pseudo device and hotplug framework deals
> > > with state machine and not managed through the cpu device.
> >
> > What's the point of removing and then re-adding the sysfs attributes
> > for the cpu caches, at each hotplug off/on sequence? To me that sounds
> > inefficient and unnecessary, no?
> >
>
> Oh well, you must look at PowerPC pHyp which can change cache nodes for
> suspend/resume operation too, let alone hotplug operations. So, we can't
> assume the same CPUs with exact same caches are always hotplugged back
> in. That's may happen in embedded and other static platforms but not
> everywhere. Anyway that's my understand as why it's done in hotplug
> patch. So it's not so simple to call it inefficient and unnecessary IMO.

Okay, I see.

>
> > If you avoid this, would that solve this problem?
> >
>
> May be, but as mentioned above we can't really. Also this change will
> help to avoid creating unnecessary power sysfs which is mainly runtime
> pm related for some of the devices created. CPU/caches was just one
> example which triggered this, but this can be more useful. We can avoid
> adding them to dpm list.

Well, to me the approach you suggest sounds prone to errors and I am
afraid people may abuse it. Moreover, I don't know if there is other
problems with it, let's see what Rafael thinks about it.

Instead I think we should make the PM core to deal with this scenario,
as all it boils down to, is to allow a device to be unregistered and
registered during system suspend/resume, with a parent device that is
"persistent" during the sequence.

Perhaps we could even just drop the corresponding printed warning,
"cache: parent cpu1 should not be sleeping", in device_pm_add() as I
wonder if it's really a necessary print.

Kind regards
Uffe