Re: [PATCH v24 01/10] drivers/base: refactor cpu.c to use .is_visible()

[Eric DeVolder <eric.devolder@oracle.com>]

I still need to convert the ifdefs within the functions to IS_ENABLED(), my apologies.
eric

On 6/28/23 13:52, Eric DeVolder wrote:
> Greg Kroah-Hartman requested that this file use the .is_visible()
> method instead of #ifdefs for the attributes in cpu.c.
> 
>   static struct attribute *cpu_root_attrs[] = {
>   #ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>      &dev_attr_probe.attr,
>      &dev_attr_release.attr,
>   #endif
>      &cpu_attrs[0].attr.attr,
>      &cpu_attrs[1].attr.attr,
>      &cpu_attrs[2].attr.attr,
>      &dev_attr_kernel_max.attr,
>      &dev_attr_offline.attr,
>      &dev_attr_isolated.attr,
>   #ifdef CONFIG_NO_HZ_FULL
>      &dev_attr_nohz_full.attr,
>   #endif
>   #ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>      &dev_attr_modalias.attr,
>   #endif
>      NULL
>   };
> 
> To that end:
>   - the .is_visible() method is implemented, and IS_ENABLED(), rather
>     than #ifdef, is used to determine the visibility of the attribute.
>   - the DEVICE_ATTR() attributes are moved outside of #ifdefs, so that
>     those structs are always present for the cpu_root_attrs[].
>   - the #ifdefs guarding the attributes in the cpu_root_attrs[] are moved
>     to the corresponding callback function; as the callback function must
>     exist now that the attribute is always compiled-in (though not
>     necessarily visible).
> 
> No functionality change intended.
> 
> Signed-off-by: Eric DeVolder <eric.devolder@oracle.com>
> ---
>   drivers/base/cpu.c | 67 ++++++++++++++++++++++++++++++++++++----------
>   1 file changed, 53 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index c1815b9dae68..75fa46a567a1 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -82,13 +82,14 @@ void unregister_cpu(struct cpu *cpu)
>   	per_cpu(cpu_sys_devices, logical_cpu) = NULL;
>   	return;
>   }
> +#endif /* CONFIG_HOTPLUG_CPU */
>   
> -#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>   static ssize_t cpu_probe_store(struct device *dev,
>   			       struct device_attribute *attr,
>   			       const char *buf,
>   			       size_t count)
>   {
> +#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>   	ssize_t cnt;
>   	int ret;
>   
> @@ -100,6 +101,9 @@ static ssize_t cpu_probe_store(struct device *dev,
>   
>   	unlock_device_hotplug();
>   	return cnt;
> +#else
> +	return 0;
> +#endif
>   }
>   
>   static ssize_t cpu_release_store(struct device *dev,
> @@ -107,6 +111,7 @@ static ssize_t cpu_release_store(struct device *dev,
>   				 const char *buf,
>   				 size_t count)
>   {
> +#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>   	ssize_t cnt;
>   	int ret;
>   
> @@ -118,12 +123,13 @@ static ssize_t cpu_release_store(struct device *dev,
>   
>   	unlock_device_hotplug();
>   	return cnt;
> +#else
> +	return 0;
> +#endif
>   }
>   
>   static DEVICE_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
>   static DEVICE_ATTR(release, S_IWUSR, NULL, cpu_release_store);
> -#endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
> -#endif /* CONFIG_HOTPLUG_CPU */
>   
>   #ifdef CONFIG_KEXEC
>   #include <linux/kexec.h>
> @@ -273,14 +279,16 @@ static ssize_t print_cpus_isolated(struct device *dev,
>   }
>   static DEVICE_ATTR(isolated, 0444, print_cpus_isolated, NULL);
>   
> -#ifdef CONFIG_NO_HZ_FULL
>   static ssize_t print_cpus_nohz_full(struct device *dev,
>   				    struct device_attribute *attr, char *buf)
>   {
> +#ifdef CONFIG_NO_HZ_FULL
>   	return sysfs_emit(buf, "%*pbl\n", cpumask_pr_args(tick_nohz_full_mask));
> +#else
> +	return 0;
> +#endif
>   }
>   static DEVICE_ATTR(nohz_full, 0444, print_cpus_nohz_full, NULL);
> -#endif
>   
>   static void cpu_device_release(struct device *dev)
>   {
> @@ -301,12 +309,12 @@ static void cpu_device_release(struct device *dev)
>   	 */
>   }
>   
> -#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>   static ssize_t print_cpu_modalias(struct device *dev,
>   				  struct device_attribute *attr,
>   				  char *buf)
>   {
>   	int len = 0;
> +#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>   	u32 i;
>   
>   	len += sysfs_emit_at(buf, len,
> @@ -322,9 +330,11 @@ static ssize_t print_cpu_modalias(struct device *dev,
>   			len += sysfs_emit_at(buf, len, ",%04X", i);
>   		}
>   	len += sysfs_emit_at(buf, len, "\n");
> +#endif
>   	return len;
>   }
>   
> +#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>   static int cpu_uevent(const struct device *dev, struct kobj_uevent_env *env)
>   {
>   	char *buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> @@ -451,32 +461,61 @@ struct device *cpu_device_create(struct device *parent, void *drvdata,
>   }
>   EXPORT_SYMBOL_GPL(cpu_device_create);
>   
> -#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>   static DEVICE_ATTR(modalias, 0444, print_cpu_modalias, NULL);
> -#endif
>   
>   static struct attribute *cpu_root_attrs[] = {
> -#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
>   	&dev_attr_probe.attr,
>   	&dev_attr_release.attr,
> -#endif
>   	&cpu_attrs[0].attr.attr,
>   	&cpu_attrs[1].attr.attr,
>   	&cpu_attrs[2].attr.attr,
>   	&dev_attr_kernel_max.attr,
>   	&dev_attr_offline.attr,
>   	&dev_attr_isolated.attr,
> -#ifdef CONFIG_NO_HZ_FULL
>   	&dev_attr_nohz_full.attr,
> -#endif
> -#ifdef CONFIG_GENERIC_CPU_AUTOPROBE
>   	&dev_attr_modalias.attr,
> -#endif
>   	NULL
>   };
>   
> +static umode_t
> +cpu_root_attr_is_visible(struct kobject *kobj,
> +			       struct attribute *attr, int unused)
> +{
> +	umode_t mode = attr->mode;
> +
> +	if (IS_ENABLED(CONFIG_ARCH_CPU_PROBE_RELEASE)) {
> +		if (attr == &dev_attr_probe.attr)
> +			return mode;
> +		if (attr == &dev_attr_release.attr)
> +			return mode;
> +	}
> +	if (attr == &cpu_attrs[0].attr.attr)
> +		return mode;
> +	if (attr == &cpu_attrs[1].attr.attr)
> +		return mode;
> +	if (attr == &cpu_attrs[2].attr.attr)
> +		return mode;
> +	if (attr == &dev_attr_kernel_max.attr)
> +		return mode;
> +	if (attr == &dev_attr_offline.attr)
> +		return mode;
> +	if (attr == &dev_attr_isolated.attr)
> +		return mode;
> +	if (IS_ENABLED(CONFIG_NO_HZ_FULL)) {
> +		if (attr == &dev_attr_nohz_full.attr)
> +			return mode;
> +	}
> +	if (IS_ENABLED(CONFIG_GENERIC_CPU_AUTOPROBE)) {
> +		if (attr == &dev_attr_modalias.attr)
> +			return mode;
> +	}
> +
> +	return 0;
> +}
> +
>   static const struct attribute_group cpu_root_attr_group = {
>   	.attrs = cpu_root_attrs,
> +	.is_visible = cpu_root_attr_is_visible,
>   };
>   
>   static const struct attribute_group *cpu_root_attr_groups[] = {