linux-kernel - Re: [RFC/RFT][PATCH v2 6/7] PM / runtime: Use device links

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: <11278470.eJh5IF8LiU@vostro.rjw.lan>
Date:   Wed, 14 Sep 2016 03:19:01 +0200
From:   "Rafael J. Wysocki" <rjw@...ysocki.net>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Linux PM list <linux-pm@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        Mark Brown <broonie@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        "Luis R. Rodriguez" <mcgrof@...e.com>
Subject: Re: [RFC/RFT][PATCH v2 6/7] PM / runtime: Use device links

On Monday, September 12, 2016 03:57:02 PM Rafael J. Wysocki wrote:
> On Monday, September 12, 2016 11:47:58 AM Lukas Wunner wrote:
> > On Thu, Sep 08, 2016 at 11:30:26PM +0200, Rafael J. Wysocki wrote:
> > > Modify the runtime PM framework to use device links to ensure that
> > > supplier devices will not be suspended if any of their consumer
> > > devices are active.
> > 
> > I think it's inconsistent to runtime resume/suspend suppliers in
> > __rpm_callback() whereas the parent is treated in rpm_suspend()
> > and rpm_resume().
> 
> The reason why I did that this way is the rollback needed in case of
> errors and that led to duplicated code if done elsewhere.
> 
> > Instead I'd suggest to amend struct dev_pm_ops with:
> > 	atomic_t		consumer_count;
> > 
> > Amend rpm_check_suspend_allowed() with:
> > 	else if (atomic_read(&dev->power.consumer_count) > 0)
> > 		retval = -EBUSY;
> 
> That is a good idea, though (from the first look at least).

No, I actually don't like it.

Suppliers are not parents and even the separate counter we have for
parents is somewhat artificial.

For parents it may be argued that they are always there, so handling them
in a special way is justified, but for suppliers I really don't see why
adding a new counter would be better.

Thanks,
Rafael