[v24,02/10] drivers/base: refactor memory.c to use .is_visible()

Message ID 20230628185215.40707-3-eric.devolder@oracle.com
State New
Headers
Series crash: Kernel handling of CPU and memory hot un/plug |

Commit Message

Eric DeVolder June 28, 2023, 6:52 p.m. UTC
  Greg Kroah-Hartman requested that this file use the .is_visible()
method instead of #ifdefs for the attributes in memory.c.

 static struct attribute *memory_memblk_attrs[] = {
     &dev_attr_phys_index.attr,
     &dev_attr_state.attr,
     &dev_attr_phys_device.attr,
     &dev_attr_removable.attr,
 #ifdef CONFIG_MEMORY_HOTREMOVE
     &dev_attr_valid_zones.attr,
 #endif
     NULL
 };

and

 static struct attribute *memory_root_attrs[] = {
 #ifdef CONFIG_ARCH_MEMORY_PROBE
     &dev_attr_probe.attr,
 #endif

 #ifdef CONFIG_MEMORY_FAILURE
     &dev_attr_soft_offline_page.attr,
     &dev_attr_hard_offline_page.attr,
 #endif

     &dev_attr_block_size_bytes.attr,
     &dev_attr_auto_online_blocks.attr,
     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_xx() attributes are moved outside of #ifdefs, so that
   those structs are always present for the memory_memblk_attrs[] and
   memory_root_attrs[].
 - the #ifdefs guarding the attributes in the memory_memblk_attrs[] and
   memory_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/memory.c | 78 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 13 deletions(-)
  

Comments

Eric DeVolder June 29, 2023, 1:25 p.m. UTC | #1
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 memory.c.
> 
>   static struct attribute *memory_memblk_attrs[] = {
>       &dev_attr_phys_index.attr,
>       &dev_attr_state.attr,
>       &dev_attr_phys_device.attr,
>       &dev_attr_removable.attr,
>   #ifdef CONFIG_MEMORY_HOTREMOVE
>       &dev_attr_valid_zones.attr,
>   #endif
>       NULL
>   };
> 
> and
> 
>   static struct attribute *memory_root_attrs[] = {
>   #ifdef CONFIG_ARCH_MEMORY_PROBE
>       &dev_attr_probe.attr,
>   #endif
> 
>   #ifdef CONFIG_MEMORY_FAILURE
>       &dev_attr_soft_offline_page.attr,
>       &dev_attr_hard_offline_page.attr,
>   #endif
> 
>       &dev_attr_block_size_bytes.attr,
>       &dev_attr_auto_online_blocks.attr,
>       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_xx() attributes are moved outside of #ifdefs, so that
>     those structs are always present for the memory_memblk_attrs[] and
>     memory_root_attrs[].
>   - the #ifdefs guarding the attributes in the memory_memblk_attrs[] and
>     memory_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/memory.c | 78 +++++++++++++++++++++++++++++++++++--------
>   1 file changed, 65 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
> index b456ac213610..f03eda7e1c9c 100644
> --- a/drivers/base/memory.c
> +++ b/drivers/base/memory.c
> @@ -405,10 +405,12 @@ static int print_allowed_zone(char *buf, int len, int nid,
>   
>   	return sysfs_emit_at(buf, len, " %s", zone->name);
>   }
> +#endif
>   
>   static ssize_t valid_zones_show(struct device *dev,
>   				struct device_attribute *attr, char *buf)
>   {
> +#ifdef CONFIG_MEMORY_HOTREMOVE
>   	struct memory_block *mem = to_memory_block(dev);
>   	unsigned long start_pfn = section_nr_to_pfn(mem->start_section_nr);
>   	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
> @@ -444,9 +446,11 @@ static ssize_t valid_zones_show(struct device *dev,
>   out:
>   	len += sysfs_emit_at(buf, len, "\n");
>   	return len;
> +#else
> +	return 0;
> +#endif
>   }
>   static DEVICE_ATTR_RO(valid_zones);
> -#endif
>   
>   static DEVICE_ATTR_RO(phys_index);
>   static DEVICE_ATTR_RW(state);
> @@ -496,10 +500,10 @@ static DEVICE_ATTR_RW(auto_online_blocks);
>    * as well as ppc64 will do all of their discovery in userspace
>    * and will require this interface.
>    */
> -#ifdef CONFIG_ARCH_MEMORY_PROBE
>   static ssize_t probe_store(struct device *dev, struct device_attribute *attr,
>   			   const char *buf, size_t count)
>   {
> +#ifdef CONFIG_ARCH_MEMORY_PROBE
>   	u64 phys_addr;
>   	int nid, ret;
>   	unsigned long pages_per_block = PAGES_PER_SECTION * sections_per_block;
> @@ -527,12 +531,13 @@ static ssize_t probe_store(struct device *dev, struct device_attribute *attr,
>   out:
>   	unlock_device_hotplug();
>   	return ret;
> +#else
> +	return 0;
> +#endif
>   }
>   
>   static DEVICE_ATTR_WO(probe);
> -#endif
>   
> -#ifdef CONFIG_MEMORY_FAILURE
>   /*
>    * Support for offlining pages of memory
>    */
> @@ -542,6 +547,7 @@ static ssize_t soft_offline_page_store(struct device *dev,
>   				       struct device_attribute *attr,
>   				       const char *buf, size_t count)
>   {
> +#ifdef CONFIG_MEMORY_FAILURE
>   	int ret;
>   	u64 pfn;
>   	if (!capable(CAP_SYS_ADMIN))
> @@ -551,6 +557,9 @@ static ssize_t soft_offline_page_store(struct device *dev,
>   	pfn >>= PAGE_SHIFT;
>   	ret = soft_offline_page(pfn, 0);
>   	return ret == 0 ? count : ret;
> +#else
> +	return 0;
> +#endif
>   }
>   
>   /* Forcibly offline a page, including killing processes. */
> @@ -558,6 +567,7 @@ static ssize_t hard_offline_page_store(struct device *dev,
>   				       struct device_attribute *attr,
>   				       const char *buf, size_t count)
>   {
> +#ifdef CONFIG_MEMORY_FAILURE
>   	int ret;
>   	u64 pfn;
>   	if (!capable(CAP_SYS_ADMIN))
> @@ -569,11 +579,13 @@ static ssize_t hard_offline_page_store(struct device *dev,
>   	if (ret == -EOPNOTSUPP)
>   		ret = 0;
>   	return ret ? ret : count;
> +#else
> +	return 0;
> +#endif
>   }
>   
>   static DEVICE_ATTR_WO(soft_offline_page);
>   static DEVICE_ATTR_WO(hard_offline_page);
> -#endif
>   
>   /* See phys_device_show(). */
>   int __weak arch_get_memory_phys_device(unsigned long start_pfn)
> @@ -611,14 +623,35 @@ static struct attribute *memory_memblk_attrs[] = {
>   	&dev_attr_state.attr,
>   	&dev_attr_phys_device.attr,
>   	&dev_attr_removable.attr,
> -#ifdef CONFIG_MEMORY_HOTREMOVE
>   	&dev_attr_valid_zones.attr,
> -#endif
>   	NULL
>   };
>   
> +static umode_t
> +memory_memblk_attr_is_visible(struct kobject *kobj,
> +			       struct attribute *attr, int unused)
> +{
> +	umode_t mode = attr->mode;
> +
> +	if (attr == &dev_attr_phys_index.attr)
> +		return mode;
> +	if (attr == &dev_attr_state.attr)
> +		return mode;
> +	if (attr == &dev_attr_phys_device.attr)
> +		return mode;
> +	if (attr == &dev_attr_removable.attr)
> +		return mode;
> +	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE)) {
> +		if (attr == &dev_attr_valid_zones.attr)
> +			return mode;
> +	}
> +
> +	return 0;
> +}
> +
>   static const struct attribute_group memory_memblk_attr_group = {
>   	.attrs = memory_memblk_attrs,
> +	.is_visible = memory_memblk_attr_is_visible,
>   };
>   
>   static const struct attribute_group *memory_memblk_attr_groups[] = {
> @@ -878,22 +911,41 @@ void remove_memory_block_devices(unsigned long start, unsigned long size)
>   }
>   
>   static struct attribute *memory_root_attrs[] = {
> -#ifdef CONFIG_ARCH_MEMORY_PROBE
>   	&dev_attr_probe.attr,
> -#endif
> -
> -#ifdef CONFIG_MEMORY_FAILURE
>   	&dev_attr_soft_offline_page.attr,
>   	&dev_attr_hard_offline_page.attr,
> -#endif
> -
>   	&dev_attr_block_size_bytes.attr,
>   	&dev_attr_auto_online_blocks.attr,
>   	NULL
>   };
>   
> +static umode_t
> +memory_root_attr_is_visible(struct kobject *kobj,
> +			       struct attribute *attr, int unused)
> +{
> +	umode_t mode = attr->mode;
> +
> +	if (IS_ENABLED(CONFIG_ARCH_MEMORY_PROBE)) {
> +		if (attr == &dev_attr_probe.attr)
> +			return mode;
> +	}
> +	if (IS_ENABLED(CONFIG_MEMORY_FAILURE)) {
> +		if (attr == &dev_attr_soft_offline_page.attr)
> +			return mode;
> +		if (attr == &dev_attr_hard_offline_page.attr)
> +			return mode;
> +	}
> +	if (attr == &dev_attr_block_size_bytes.attr)
> +		return mode;
> +	if (attr == &dev_attr_auto_online_blocks.attr)
> +		return mode;
> +
> +	return 0;
> +}
> +
>   static const struct attribute_group memory_root_attr_group = {
>   	.attrs = memory_root_attrs,
> +	.is_visible = memory_root_attr_is_visible,
>   };
>   
>   static const struct attribute_group *memory_root_attr_groups[] = {
  

Patch

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index b456ac213610..f03eda7e1c9c 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -405,10 +405,12 @@  static int print_allowed_zone(char *buf, int len, int nid,
 
 	return sysfs_emit_at(buf, len, " %s", zone->name);
 }
+#endif
 
 static ssize_t valid_zones_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
+#ifdef CONFIG_MEMORY_HOTREMOVE
 	struct memory_block *mem = to_memory_block(dev);
 	unsigned long start_pfn = section_nr_to_pfn(mem->start_section_nr);
 	unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
@@ -444,9 +446,11 @@  static ssize_t valid_zones_show(struct device *dev,
 out:
 	len += sysfs_emit_at(buf, len, "\n");
 	return len;
+#else
+	return 0;
+#endif
 }
 static DEVICE_ATTR_RO(valid_zones);
-#endif
 
 static DEVICE_ATTR_RO(phys_index);
 static DEVICE_ATTR_RW(state);
@@ -496,10 +500,10 @@  static DEVICE_ATTR_RW(auto_online_blocks);
  * as well as ppc64 will do all of their discovery in userspace
  * and will require this interface.
  */
-#ifdef CONFIG_ARCH_MEMORY_PROBE
 static ssize_t probe_store(struct device *dev, struct device_attribute *attr,
 			   const char *buf, size_t count)
 {
+#ifdef CONFIG_ARCH_MEMORY_PROBE
 	u64 phys_addr;
 	int nid, ret;
 	unsigned long pages_per_block = PAGES_PER_SECTION * sections_per_block;
@@ -527,12 +531,13 @@  static ssize_t probe_store(struct device *dev, struct device_attribute *attr,
 out:
 	unlock_device_hotplug();
 	return ret;
+#else
+	return 0;
+#endif
 }
 
 static DEVICE_ATTR_WO(probe);
-#endif
 
-#ifdef CONFIG_MEMORY_FAILURE
 /*
  * Support for offlining pages of memory
  */
@@ -542,6 +547,7 @@  static ssize_t soft_offline_page_store(struct device *dev,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
+#ifdef CONFIG_MEMORY_FAILURE
 	int ret;
 	u64 pfn;
 	if (!capable(CAP_SYS_ADMIN))
@@ -551,6 +557,9 @@  static ssize_t soft_offline_page_store(struct device *dev,
 	pfn >>= PAGE_SHIFT;
 	ret = soft_offline_page(pfn, 0);
 	return ret == 0 ? count : ret;
+#else
+	return 0;
+#endif
 }
 
 /* Forcibly offline a page, including killing processes. */
@@ -558,6 +567,7 @@  static ssize_t hard_offline_page_store(struct device *dev,
 				       struct device_attribute *attr,
 				       const char *buf, size_t count)
 {
+#ifdef CONFIG_MEMORY_FAILURE
 	int ret;
 	u64 pfn;
 	if (!capable(CAP_SYS_ADMIN))
@@ -569,11 +579,13 @@  static ssize_t hard_offline_page_store(struct device *dev,
 	if (ret == -EOPNOTSUPP)
 		ret = 0;
 	return ret ? ret : count;
+#else
+	return 0;
+#endif
 }
 
 static DEVICE_ATTR_WO(soft_offline_page);
 static DEVICE_ATTR_WO(hard_offline_page);
-#endif
 
 /* See phys_device_show(). */
 int __weak arch_get_memory_phys_device(unsigned long start_pfn)
@@ -611,14 +623,35 @@  static struct attribute *memory_memblk_attrs[] = {
 	&dev_attr_state.attr,
 	&dev_attr_phys_device.attr,
 	&dev_attr_removable.attr,
-#ifdef CONFIG_MEMORY_HOTREMOVE
 	&dev_attr_valid_zones.attr,
-#endif
 	NULL
 };
 
+static umode_t
+memory_memblk_attr_is_visible(struct kobject *kobj,
+			       struct attribute *attr, int unused)
+{
+	umode_t mode = attr->mode;
+
+	if (attr == &dev_attr_phys_index.attr)
+		return mode;
+	if (attr == &dev_attr_state.attr)
+		return mode;
+	if (attr == &dev_attr_phys_device.attr)
+		return mode;
+	if (attr == &dev_attr_removable.attr)
+		return mode;
+	if (IS_ENABLED(CONFIG_MEMORY_HOTREMOVE)) {
+		if (attr == &dev_attr_valid_zones.attr)
+			return mode;
+	}
+
+	return 0;
+}
+
 static const struct attribute_group memory_memblk_attr_group = {
 	.attrs = memory_memblk_attrs,
+	.is_visible = memory_memblk_attr_is_visible,
 };
 
 static const struct attribute_group *memory_memblk_attr_groups[] = {
@@ -878,22 +911,41 @@  void remove_memory_block_devices(unsigned long start, unsigned long size)
 }
 
 static struct attribute *memory_root_attrs[] = {
-#ifdef CONFIG_ARCH_MEMORY_PROBE
 	&dev_attr_probe.attr,
-#endif
-
-#ifdef CONFIG_MEMORY_FAILURE
 	&dev_attr_soft_offline_page.attr,
 	&dev_attr_hard_offline_page.attr,
-#endif
-
 	&dev_attr_block_size_bytes.attr,
 	&dev_attr_auto_online_blocks.attr,
 	NULL
 };
 
+static umode_t
+memory_root_attr_is_visible(struct kobject *kobj,
+			       struct attribute *attr, int unused)
+{
+	umode_t mode = attr->mode;
+
+	if (IS_ENABLED(CONFIG_ARCH_MEMORY_PROBE)) {
+		if (attr == &dev_attr_probe.attr)
+			return mode;
+	}
+	if (IS_ENABLED(CONFIG_MEMORY_FAILURE)) {
+		if (attr == &dev_attr_soft_offline_page.attr)
+			return mode;
+		if (attr == &dev_attr_hard_offline_page.attr)
+			return mode;
+	}
+	if (attr == &dev_attr_block_size_bytes.attr)
+		return mode;
+	if (attr == &dev_attr_auto_online_blocks.attr)
+		return mode;
+
+	return 0;
+}
+
 static const struct attribute_group memory_root_attr_group = {
 	.attrs = memory_root_attrs,
+	.is_visible = memory_root_attr_is_visible,
 };
 
 static const struct attribute_group *memory_root_attr_groups[] = {