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

Message-ID: <20220325164255.GA12710@wunner.de>
Date:   Fri, 25 Mar 2022 17:42:55 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
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 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.

That way you only need the if/else ladder once in your patch.

For an example, see pcie_port_device_iter() and its callers
pcie_port_device_suspend() and so on:

https://elixir.bootlin.com/linux/latest/source/drivers/pci/pcie/portdrv_core.c#L372

Thanks,

Lukas