linux-kernel - Re: [PATCHv2 1/3] software node: Power management operations for software nodes

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

Message-ID: <YkFuiarj9z9YUgOs@kuha.fi.intel.com>
Date:   Mon, 28 Mar 2022 11:15:05 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Felipe Balbi <balbi@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-acpi@...r.kernel.org
Subject: Re: [PATCHv2 1/3] software node: Power management operations for
 software nodes

Hi Lukas,

On Fri, Mar 25, 2022 at 05:42:55PM +0100, Lukas Wunner wrote:
> Hi Heikki,
> 
> saw this linked in your WSR and felt compelled to reply... ;)
> 
> On Thu, Oct 29, 2020 at 01:59:39PM +0300, Heikki Krogerus wrote:
> > +static int software_node_runtime_suspend(struct device *dev)
> > +{
> > +	struct swnode_pm_domain *domain = to_swnode_pm_domain(dev->pm_domain);
> > +	struct swnode *swnode = dev_to_swnode(dev);
> > +	int ret;
> > +
> > +	if (domain->primary && domain->primary->ops.runtime_suspend)
> > +		ret = domain->primary->ops.runtime_suspend(dev);
> > +	else if (dev->type && dev->type->pm && dev->type->pm->runtime_suspend)
> > +		ret = dev->type->pm->runtime_suspend(dev);
> > +	else if (dev->class && dev->class->pm && dev->class->pm->runtime_suspend)
> > +		ret = dev->class->pm->runtime_suspend(dev);
> > +	else if (dev->bus && dev->bus->pm && dev->bus->pm->runtime_suspend)
> > +		ret = dev->bus->pm->runtime_suspend(dev);
> > +	else
> > +		ret = pm_generic_runtime_suspend(dev);
> 
> This if/else ladder seems to be duplicated for every single PM callback
> in this patch.
> 
> Code size can be reduced significantly if you use offsetof() to determine
> the offset of the given callback in struct pm_ops, then pass that offset
> to a helper which contains the above-quoted if/else ladder and retrieves
> the callback.  Finally invoke the callback you've just retrieved.

I think Sakari already suggested that. I'll improve this part in the
next version.

thanks,

-- 
heikki