diff mbox series

[RFC,v1,30/57] drivers/base: Remove PAGE_SIZE compile-time constant assumption

Message ID 20241014105912.3207374-30-ryan.roberts@arm.com (mailing list archive)
State New
Headers show
Series Boot-time page size selection for arm64 | expand

Commit Message

Ryan Roberts Oct. 14, 2024, 10:58 a.m. UTC 
To prepare for supporting boot-time page size selection, refactor code
to remove assumptions about PAGE_SIZE being compile-time constant. Code
intended to be equivalent when compile-time page size is active.

Update BUILD_BUG_ON() to test against page size limits.

CPUMAP_FILE_MAX_BYTES and CPULIST_FILE_MAX_BYTES are both defined
relative to PAGE_SIZE, so when these values are assigned to global
variables via BIN_ATTR_RO(), let's wrap them with
DEFINE_GLOBAL_PAGE_SIZE_VAR() so that their assignment can be deferred
until boot-time.

Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
---

***NOTE***
Any confused maintainers may want to read the cover note here for context:
https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/

 drivers/base/node.c     |  6 +++---
 drivers/base/topology.c | 32 ++++++++++++++++----------------
 include/linux/cpumask.h |  5 +++++
 3 files changed, 24 insertions(+), 19 deletions(-)

Comments

Ryan Roberts Oct. 16, 2024, 2:45 p.m. UTC | #1 
+ Greg Kroah-Hartman

This was a rather tricky series to get the recipients correct for and my script
did not realize that "supporter" was a pseudonym for "maintainer" so you were
missed off the original post. Appologies!

More context in cover letter:
https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/


On 14/10/2024 11:58, Ryan Roberts wrote:
> To prepare for supporting boot-time page size selection, refactor code
> to remove assumptions about PAGE_SIZE being compile-time constant. Code
> intended to be equivalent when compile-time page size is active.
> 
> Update BUILD_BUG_ON() to test against page size limits.
> 
> CPUMAP_FILE_MAX_BYTES and CPULIST_FILE_MAX_BYTES are both defined
> relative to PAGE_SIZE, so when these values are assigned to global
> variables via BIN_ATTR_RO(), let's wrap them with
> DEFINE_GLOBAL_PAGE_SIZE_VAR() so that their assignment can be deferred
> until boot-time.
> 
> Signed-off-by: Ryan Roberts <ryan.roberts@arm.com>
> ---
> 
> ***NOTE***
> Any confused maintainers may want to read the cover note here for context:
> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/
> 
>  drivers/base/node.c     |  6 +++---
>  drivers/base/topology.c | 32 ++++++++++++++++----------------
>  include/linux/cpumask.h |  5 +++++
>  3 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index eb72580288e62..30e6549e4c438 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -45,7 +45,7 @@ static inline ssize_t cpumap_read(struct file *file, struct kobject *kobj,
>  	return n;
>  }
>  
> -static BIN_ATTR_RO(cpumap, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(cpumap, CPUMAP_FILE_MAX_BYTES);
>  
>  static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
>  				   struct bin_attribute *attr, char *buf,
> @@ -66,7 +66,7 @@ static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
>  	return n;
>  }
>  
> -static BIN_ATTR_RO(cpulist, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(cpulist, CPULIST_FILE_MAX_BYTES);
>  
>  /**
>   * struct node_access_nodes - Access class device to hold user visible
> @@ -558,7 +558,7 @@ static ssize_t node_read_distance(struct device *dev,
>  	 * buf is currently PAGE_SIZE in length and each node needs 4 chars
>  	 * at the most (distance + space or newline).
>  	 */
> -	BUILD_BUG_ON(MAX_NUMNODES * 4 > PAGE_SIZE);
> +	BUILD_BUG_ON(MAX_NUMNODES * 4 > PAGE_SIZE_MIN);
>  
>  	for_each_online_node(i) {
>  		len += sysfs_emit_at(buf, len, "%s%d",
> diff --git a/drivers/base/topology.c b/drivers/base/topology.c
> index 89f98be5c5b99..bdbdbefd95b15 100644
> --- a/drivers/base/topology.c
> +++ b/drivers/base/topology.c
> @@ -62,47 +62,47 @@ define_id_show_func(ppin, "0x%llx");
>  static DEVICE_ATTR_ADMIN_RO(ppin);
>  
>  define_siblings_read_func(thread_siblings, sibling_cpumask);
> -static BIN_ATTR_RO(thread_siblings, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(thread_siblings_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(thread_siblings, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(thread_siblings_list, CPULIST_FILE_MAX_BYTES);
>  
>  define_siblings_read_func(core_cpus, sibling_cpumask);
> -static BIN_ATTR_RO(core_cpus, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(core_cpus_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(core_cpus, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(core_cpus_list, CPULIST_FILE_MAX_BYTES);
>  
>  define_siblings_read_func(core_siblings, core_cpumask);
> -static BIN_ATTR_RO(core_siblings, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(core_siblings_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(core_siblings, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(core_siblings_list, CPULIST_FILE_MAX_BYTES);
>  
>  #ifdef TOPOLOGY_CLUSTER_SYSFS
>  define_siblings_read_func(cluster_cpus, cluster_cpumask);
> -static BIN_ATTR_RO(cluster_cpus, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(cluster_cpus_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(cluster_cpus, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(cluster_cpus_list, CPULIST_FILE_MAX_BYTES);
>  #endif
>  
>  #ifdef TOPOLOGY_DIE_SYSFS
>  define_siblings_read_func(die_cpus, die_cpumask);
> -static BIN_ATTR_RO(die_cpus, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(die_cpus_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(die_cpus, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(die_cpus_list, CPULIST_FILE_MAX_BYTES);
>  #endif
>  
>  define_siblings_read_func(package_cpus, core_cpumask);
> -static BIN_ATTR_RO(package_cpus, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(package_cpus_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(package_cpus, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(package_cpus_list, CPULIST_FILE_MAX_BYTES);
>  
>  #ifdef TOPOLOGY_BOOK_SYSFS
>  define_id_show_func(book_id, "%d");
>  static DEVICE_ATTR_RO(book_id);
>  define_siblings_read_func(book_siblings, book_cpumask);
> -static BIN_ATTR_RO(book_siblings, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(book_siblings_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(book_siblings, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(book_siblings_list, CPULIST_FILE_MAX_BYTES);
>  #endif
>  
>  #ifdef TOPOLOGY_DRAWER_SYSFS
>  define_id_show_func(drawer_id, "%d");
>  static DEVICE_ATTR_RO(drawer_id);
>  define_siblings_read_func(drawer_siblings, drawer_cpumask);
> -static BIN_ATTR_RO(drawer_siblings, CPUMAP_FILE_MAX_BYTES);
> -static BIN_ATTR_RO(drawer_siblings_list, CPULIST_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(drawer_siblings, CPUMAP_FILE_MAX_BYTES);
> +static CPU_FILE_BIN_ATTR_RO(drawer_siblings_list, CPULIST_FILE_MAX_BYTES);
>  #endif
>  
>  static struct bin_attribute *bin_attrs[] = {
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 53158de44b837..f654b4198abc2 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -1292,4 +1292,9 @@ cpumap_print_list_to_buf(char *buf, const struct cpumask *mask,
>  					? (NR_CPUS * 9)/32 - 1 : PAGE_SIZE)
>  #define CPULIST_FILE_MAX_BYTES  (((NR_CPUS * 7)/2 > PAGE_SIZE) ? (NR_CPUS * 7)/2 : PAGE_SIZE)
>  
> +#define CPU_FILE_BIN_ATTR_RO(_name, _size)				\
> +	DEFINE_GLOBAL_PAGE_SIZE_VAR(struct bin_attribute,		\
> +				    bin_attr_##_name,			\
> +				    __BIN_ATTR_RO(_name, _size))
> +
>  #endif /* __LINUX_CPUMASK_H */
Greg KH Oct. 16, 2024, 3:04 p.m. UTC | #2 
On Wed, Oct 16, 2024 at 03:45:48PM +0100, Ryan Roberts wrote:
> + Greg Kroah-Hartman
> 
> This was a rather tricky series to get the recipients correct for and my script
> did not realize that "supporter" was a pseudonym for "maintainer" so you were
> missed off the original post. Appologies!

"supporter" is actually a much stronger "signal" than "maintainer"
according to the MAINTAINERS file:
		Supported:   Someone is actually paid to look after this.
		Maintained:  Someone actually looks after it.

> More context in cover letter:
> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/

Ick, good luck!

greg k-h
Ryan Roberts Oct. 16, 2024, 3:12 p.m. UTC | #3 
On 16/10/2024 16:04, Greg Kroah-Hartman wrote:
> On Wed, Oct 16, 2024 at 03:45:48PM +0100, Ryan Roberts wrote:
>> + Greg Kroah-Hartman
>>
>> This was a rather tricky series to get the recipients correct for and my script
>> did not realize that "supporter" was a pseudonym for "maintainer" so you were
>> missed off the original post. Appologies!
> 
> "supporter" is actually a much stronger "signal" than "maintainer"
> according to the MAINTAINERS file:
> 		Supported:   Someone is actually paid to look after this.
> 		Maintained:  Someone actually looks after it.

Yes, consider me educated now. For some reason my brain always thought
"supporter" was someone who was active in the subsystem but without an official
affiliation.

> 
>> More context in cover letter:
>> https://lore.kernel.org/all/20241014105514.3206191-1-ryan.roberts@arm.com/
> 
> Ick, good luck!
> 
> greg k-h
diff mbox series

Patch

diff --git a/drivers/base/node.c b/drivers/base/node.c
index eb72580288e62..30e6549e4c438 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -45,7 +45,7 @@  static inline ssize_t cpumap_read(struct file *file, struct kobject *kobj,
 	return n;
 }
 
-static BIN_ATTR_RO(cpumap, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(cpumap, CPUMAP_FILE_MAX_BYTES);
 
 static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
 				   struct bin_attribute *attr, char *buf,
@@ -66,7 +66,7 @@  static inline ssize_t cpulist_read(struct file *file, struct kobject *kobj,
 	return n;
 }
 
-static BIN_ATTR_RO(cpulist, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(cpulist, CPULIST_FILE_MAX_BYTES);
 
 /**
  * struct node_access_nodes - Access class device to hold user visible
@@ -558,7 +558,7 @@  static ssize_t node_read_distance(struct device *dev,
 	 * buf is currently PAGE_SIZE in length and each node needs 4 chars
 	 * at the most (distance + space or newline).
 	 */
-	BUILD_BUG_ON(MAX_NUMNODES * 4 > PAGE_SIZE);
+	BUILD_BUG_ON(MAX_NUMNODES * 4 > PAGE_SIZE_MIN);
 
 	for_each_online_node(i) {
 		len += sysfs_emit_at(buf, len, "%s%d",
diff --git a/drivers/base/topology.c b/drivers/base/topology.c
index 89f98be5c5b99..bdbdbefd95b15 100644
--- a/drivers/base/topology.c
+++ b/drivers/base/topology.c
@@ -62,47 +62,47 @@  define_id_show_func(ppin, "0x%llx");
 static DEVICE_ATTR_ADMIN_RO(ppin);
 
 define_siblings_read_func(thread_siblings, sibling_cpumask);
-static BIN_ATTR_RO(thread_siblings, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(thread_siblings_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(thread_siblings, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(thread_siblings_list, CPULIST_FILE_MAX_BYTES);
 
 define_siblings_read_func(core_cpus, sibling_cpumask);
-static BIN_ATTR_RO(core_cpus, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(core_cpus_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(core_cpus, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(core_cpus_list, CPULIST_FILE_MAX_BYTES);
 
 define_siblings_read_func(core_siblings, core_cpumask);
-static BIN_ATTR_RO(core_siblings, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(core_siblings_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(core_siblings, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(core_siblings_list, CPULIST_FILE_MAX_BYTES);
 
 #ifdef TOPOLOGY_CLUSTER_SYSFS
 define_siblings_read_func(cluster_cpus, cluster_cpumask);
-static BIN_ATTR_RO(cluster_cpus, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(cluster_cpus_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(cluster_cpus, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(cluster_cpus_list, CPULIST_FILE_MAX_BYTES);
 #endif
 
 #ifdef TOPOLOGY_DIE_SYSFS
 define_siblings_read_func(die_cpus, die_cpumask);
-static BIN_ATTR_RO(die_cpus, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(die_cpus_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(die_cpus, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(die_cpus_list, CPULIST_FILE_MAX_BYTES);
 #endif
 
 define_siblings_read_func(package_cpus, core_cpumask);
-static BIN_ATTR_RO(package_cpus, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(package_cpus_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(package_cpus, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(package_cpus_list, CPULIST_FILE_MAX_BYTES);
 
 #ifdef TOPOLOGY_BOOK_SYSFS
 define_id_show_func(book_id, "%d");
 static DEVICE_ATTR_RO(book_id);
 define_siblings_read_func(book_siblings, book_cpumask);
-static BIN_ATTR_RO(book_siblings, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(book_siblings_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(book_siblings, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(book_siblings_list, CPULIST_FILE_MAX_BYTES);
 #endif
 
 #ifdef TOPOLOGY_DRAWER_SYSFS
 define_id_show_func(drawer_id, "%d");
 static DEVICE_ATTR_RO(drawer_id);
 define_siblings_read_func(drawer_siblings, drawer_cpumask);
-static BIN_ATTR_RO(drawer_siblings, CPUMAP_FILE_MAX_BYTES);
-static BIN_ATTR_RO(drawer_siblings_list, CPULIST_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(drawer_siblings, CPUMAP_FILE_MAX_BYTES);
+static CPU_FILE_BIN_ATTR_RO(drawer_siblings_list, CPULIST_FILE_MAX_BYTES);
 #endif
 
 static struct bin_attribute *bin_attrs[] = {
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index 53158de44b837..f654b4198abc2 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -1292,4 +1292,9 @@  cpumap_print_list_to_buf(char *buf, const struct cpumask *mask,
 					? (NR_CPUS * 9)/32 - 1 : PAGE_SIZE)
 #define CPULIST_FILE_MAX_BYTES  (((NR_CPUS * 7)/2 > PAGE_SIZE) ? (NR_CPUS * 7)/2 : PAGE_SIZE)
 
+#define CPU_FILE_BIN_ATTR_RO(_name, _size)				\
+	DEFINE_GLOBAL_PAGE_SIZE_VAR(struct bin_attribute,		\
+				    bin_attr_##_name,			\
+				    __BIN_ATTR_RO(_name, _size))
+
 #endif /* __LINUX_CPUMASK_H */