Re: [PATCH 1/1] PCI/sysfs: Demacrofy pci_dev_resource_resize_attr(n) functions

[Bjorn Helgaas]

On Thu, Feb 22, 2024 at 01:46:06PM +0200, Ilpo Järvinen wrote:
> pci_dev_resource_resize_attr(n) macro is invoked for six resources,
> creating a large footprint function for each resource.
> 
> Rework the macro to only create a function that calls a helper function
> so the compiler can decide if it warrants to inline the function or
> not.
> 
> With x86_64 defconfig, this saves roughly 2.5kB:
> 
> $ scripts/bloat-o-meter drivers/pci/pci-sysfs.o{.old,.new}
> add/remove: 1/0 grow/shrink: 0/6 up/down: 512/-2934 (-2422)
> Function                                     old     new   delta
> __resource_resize_store                        -     512    +512
> resource5_resize_store                       503      14    -489
> resource4_resize_store                       503      14    -489
> resource3_resize_store                       503      14    -489
> resource2_resize_store                       503      14    -489
> resource1_resize_store                       503      14    -489
> resource0_resize_store                       500      11    -489
> Total: Before=13399, After=10977, chg -18.08%
> 
> (The compiler seemingly chose to still inline __resource_resize_show()
> which is fine, those functions are not very complex/large.)
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

Applied to pci/sysfs for v6.9, thanks!

> ---
>  drivers/pci/pci-sysfs.c | 138 +++++++++++++++++++++-------------------
>  1 file changed, 74 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 2321fdfefd7d..613c5fc4f0a2 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1410,79 +1410,89 @@ static const struct attribute_group pci_dev_reset_attr_group = {
>  	.is_visible = pci_dev_reset_attr_is_visible,
>  };
>  
> +static ssize_t __resource_resize_show(struct device *dev, int n, char *buf)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	ssize_t ret;
> +
> +	pci_config_pm_runtime_get(pdev);
> +
> +	ret = sysfs_emit(buf, "%016llx\n",
> +			 (u64)pci_rebar_get_possible_sizes(pdev, n));
> +
> +	pci_config_pm_runtime_put(pdev);
> +
> +	return ret;
> +}
> +
> +static ssize_t __resource_resize_store(struct device *dev, int n,
> +				       const char *buf, size_t count)
> +{
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	unsigned long size, flags;
> +	int ret, i;
> +	u16 cmd;
> +
> +	if (kstrtoul(buf, 0, &size) < 0)
> +		return -EINVAL;
> +
> +	device_lock(dev);
> +	if (dev->driver) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	pci_config_pm_runtime_get(pdev);
> +
> +	if ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) {
> +		ret = aperture_remove_conflicting_pci_devices(pdev,
> +						"resourceN_resize");
> +		if (ret)
> +			goto pm_put;
> +	}
> +
> +	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> +	pci_write_config_word(pdev, PCI_COMMAND,
> +			      cmd & ~PCI_COMMAND_MEMORY);
> +
> +	flags = pci_resource_flags(pdev, n);
> +
> +	pci_remove_resource_files(pdev);
> +
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +		if (pci_resource_len(pdev, i) &&
> +		    pci_resource_flags(pdev, i) == flags)
> +			pci_release_resource(pdev, i);
> +	}
> +
> +	ret = pci_resize_resource(pdev, n, size);
> +
> +	pci_assign_unassigned_bus_resources(pdev->bus);
> +
> +	if (pci_create_resource_files(pdev))
> +		pci_warn(pdev, "Failed to recreate resource files after BAR resizing\n");
> +
> +	pci_write_config_word(pdev, PCI_COMMAND, cmd);
> +pm_put:
> +	pci_config_pm_runtime_put(pdev);
> +unlock:
> +	device_unlock(dev);
> +
> +	return ret ? ret : count;
> +}
> +
>  #define pci_dev_resource_resize_attr(n)					\
>  static ssize_t resource##n##_resize_show(struct device *dev,		\
>  					 struct device_attribute *attr,	\
> -					 char * buf)			\
> +					 char *buf)			\
>  {									\
> -	struct pci_dev *pdev = to_pci_dev(dev);				\
> -	ssize_t ret;							\
> -									\
> -	pci_config_pm_runtime_get(pdev);				\
> -									\
> -	ret = sysfs_emit(buf, "%016llx\n",				\
> -			 (u64)pci_rebar_get_possible_sizes(pdev, n));	\
> -									\
> -	pci_config_pm_runtime_put(pdev);				\
> -									\
> -	return ret;							\
> +	return __resource_resize_show(dev, n, buf);			\
>  }									\
> -									\
>  static ssize_t resource##n##_resize_store(struct device *dev,		\
>  					  struct device_attribute *attr,\
>  					  const char *buf, size_t count)\
>  {									\
> -	struct pci_dev *pdev = to_pci_dev(dev);				\
> -	unsigned long size, flags;					\
> -	int ret, i;							\
> -	u16 cmd;							\
> -									\
> -	if (kstrtoul(buf, 0, &size) < 0)				\
> -		return -EINVAL;						\
> -									\
> -	device_lock(dev);						\
> -	if (dev->driver) {						\
> -		ret = -EBUSY;						\
> -		goto unlock;						\
> -	}								\
> -									\
> -	pci_config_pm_runtime_get(pdev);				\
> -									\
> -	if ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) {		\
> -		ret = aperture_remove_conflicting_pci_devices(pdev,	\
> -						"resourceN_resize");	\
> -		if (ret)						\
> -			goto pm_put;					\
> -	}								\
> -									\
> -	pci_read_config_word(pdev, PCI_COMMAND, &cmd);			\
> -	pci_write_config_word(pdev, PCI_COMMAND,			\
> -			      cmd & ~PCI_COMMAND_MEMORY);		\
> -									\
> -	flags = pci_resource_flags(pdev, n);				\
> -									\
> -	pci_remove_resource_files(pdev);				\
> -									\
> -	for (i = 0; i < PCI_STD_NUM_BARS; i++) {			\
> -		if (pci_resource_len(pdev, i) &&			\
> -		    pci_resource_flags(pdev, i) == flags)		\
> -			pci_release_resource(pdev, i);			\
> -	}								\
> -									\
> -	ret = pci_resize_resource(pdev, n, size);			\
> -									\
> -	pci_assign_unassigned_bus_resources(pdev->bus);			\
> -									\
> -	if (pci_create_resource_files(pdev))				\
> -		pci_warn(pdev, "Failed to recreate resource files after BAR resizing\n");\
> -									\
> -	pci_write_config_word(pdev, PCI_COMMAND, cmd);			\
> -pm_put:									\
> -	pci_config_pm_runtime_put(pdev);				\
> -unlock:									\
> -	device_unlock(dev);						\
> -									\
> -	return ret ? ret : count;					\
> +	return __resource_resize_store(dev, n, buf, count);		\
>  }									\
>  static DEVICE_ATTR_RW(resource##n##_resize)
>  
> -- 
> 2.39.2
>