[RFC PATCH V3 12/17] sysfs: Add sysfs support to tune scanning

Raghavendra K T posted 17 patches 1 month, 3 weeks ago
There is a newer version of this series
[RFC PATCH V3 12/17] sysfs: Add sysfs support to tune scanning
Posted by Raghavendra K T 1 month, 3 weeks ago
Support below tunables:
scan_enable: turn on or turn off mm_struct scanning
scan_period: initial scan_period (default: 2sec)
scan_sleep_ms: sleep time between two successive round of scanning and
migration.
mms_to_scan: total mm_struct to scan before taking a pause.
target_node: default regular node to which migration of accessed pages
is done (this is only fall back mechnism, otherwise target_node
heuristic is used).

Signed-off-by: Raghavendra K T <raghavendra.kt@amd.com>
---
 mm/kscand.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 205 insertions(+)

diff --git a/mm/kscand.c b/mm/kscand.c
index 41321d373be7..a73606f7ca3c 100644
--- a/mm/kscand.c
+++ b/mm/kscand.c
@@ -21,6 +21,7 @@
 #include <linux/delay.h>
 #include <linux/cleanup.h>
 #include <linux/minmax.h>
+#include <trace/events/kmem.h>
 
 #include <asm/pgalloc.h>
 #include "internal.h"
@@ -173,6 +174,171 @@ static bool kscand_eligible_srcnid(int nid)
 	return  !node_is_toptier(nid);
 }
 
+#ifdef CONFIG_SYSFS
+static ssize_t scan_sleep_ms_show(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 char *buf)
+{
+	return sysfs_emit(buf, "%u\n", kscand_scan_sleep_ms);
+}
+
+static ssize_t scan_sleep_ms_store(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  const char *buf, size_t count)
+{
+	unsigned int msecs;
+	int err;
+
+	err = kstrtouint(buf, 10, &msecs);
+	if (err)
+		return -EINVAL;
+
+	kscand_scan_sleep_ms = msecs;
+	kscand_sleep_expire = 0;
+	wake_up_interruptible(&kscand_wait);
+
+	return count;
+}
+
+static struct kobj_attribute scan_sleep_ms_attr =
+	__ATTR_RW(scan_sleep_ms);
+
+static ssize_t mm_scan_period_ms_show(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 char *buf)
+{
+	return sysfs_emit(buf, "%u\n", kscand_mm_scan_period_ms);
+}
+
+/* If a value less than MIN or greater than MAX asked for store value is clamped */
+static ssize_t mm_scan_period_ms_store(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  const char *buf, size_t count)
+{
+	unsigned int msecs, stored_msecs;
+	int err;
+
+	err = kstrtouint(buf, 10, &msecs);
+	if (err)
+		return -EINVAL;
+
+	stored_msecs = clamp(msecs, KSCAND_SCAN_PERIOD_MIN, KSCAND_SCAN_PERIOD_MAX);
+
+	kscand_mm_scan_period_ms = stored_msecs;
+	kscand_sleep_expire = 0;
+	wake_up_interruptible(&kscand_wait);
+
+	return count;
+}
+
+static struct kobj_attribute mm_scan_period_ms_attr =
+	__ATTR_RW(mm_scan_period_ms);
+
+static ssize_t mms_to_scan_show(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 char *buf)
+{
+	return sysfs_emit(buf, "%lu\n", kscand_mms_to_scan);
+}
+
+static ssize_t mms_to_scan_store(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  const char *buf, size_t count)
+{
+	unsigned long val;
+	int err;
+
+	err = kstrtoul(buf, 10, &val);
+	if (err)
+		return -EINVAL;
+
+	kscand_mms_to_scan = val;
+	kscand_sleep_expire = 0;
+	wake_up_interruptible(&kscand_wait);
+
+	return count;
+}
+
+static struct kobj_attribute mms_to_scan_attr =
+	__ATTR_RW(mms_to_scan);
+
+static ssize_t scan_enabled_show(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 char *buf)
+{
+	return sysfs_emit(buf, "%u\n", kscand_scan_enabled ? 1 : 0);
+}
+
+static ssize_t scan_enabled_store(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  const char *buf, size_t count)
+{
+	unsigned int val;
+	int err;
+
+	err = kstrtouint(buf, 10, &val);
+	if (err || val > 1)
+		return -EINVAL;
+
+	if (val) {
+		kscand_scan_enabled = true;
+		need_wakeup = true;
+	} else
+		kscand_scan_enabled = false;
+
+	kscand_sleep_expire = 0;
+	wake_up_interruptible(&kscand_wait);
+
+	return count;
+}
+
+static struct kobj_attribute scan_enabled_attr =
+	__ATTR_RW(scan_enabled);
+
+static ssize_t target_node_show(struct kobject *kobj,
+					 struct kobj_attribute *attr,
+					 char *buf)
+{
+	return sysfs_emit(buf, "%u\n", kscand_target_node);
+}
+
+static ssize_t target_node_store(struct kobject *kobj,
+					  struct kobj_attribute *attr,
+					  const char *buf, size_t count)
+{
+	int err, node;
+
+	err = kstrtoint(buf, 10, &node);
+	if (err)
+		return -EINVAL;
+
+	kscand_sleep_expire = 0;
+	if (!node_is_toptier(node))
+		return -EINVAL;
+
+	kscand_target_node = node;
+	wake_up_interruptible(&kscand_wait);
+
+	return count;
+}
+static struct kobj_attribute target_node_attr =
+	__ATTR_RW(target_node);
+
+static struct attribute *kscand_attr[] = {
+	&scan_sleep_ms_attr.attr,
+	&mm_scan_period_ms_attr.attr,
+	&mms_to_scan_attr.attr,
+	&scan_enabled_attr.attr,
+	&target_node_attr.attr,
+	NULL,
+};
+
+struct attribute_group kscand_attr_group = {
+	.attrs = kscand_attr,
+	.name = "kscand",
+};
+#endif
+
 static inline int kscand_has_work(void)
 {
 	return !list_empty(&kscand_scan.mm_head);
@@ -1231,11 +1397,45 @@ static int kscand(void *none)
 	return 0;
 }
 
+#ifdef CONFIG_SYSFS
+extern struct kobject *mm_kobj;
+static int __init kscand_init_sysfs(struct kobject **kobj)
+{
+	int err;
+
+	err = sysfs_create_group(*kobj, &kscand_attr_group);
+	if (err) {
+		pr_err("failed to register kscand group\n");
+		goto err_kscand_attr;
+	}
+
+	return 0;
+
+err_kscand_attr:
+	sysfs_remove_group(*kobj, &kscand_attr_group);
+	return err;
+}
+
+static void __init kscand_exit_sysfs(struct kobject *kobj)
+{
+		sysfs_remove_group(kobj, &kscand_attr_group);
+}
+#else
+static inline int __init kscand_init_sysfs(struct kobject **kobj)
+{
+	return 0;
+}
+static inline void __init kscand_exit_sysfs(struct kobject *kobj)
+{
+}
+#endif
+
 static inline void kscand_destroy(void)
 {
 	kmem_cache_destroy(kscand_slot_cache);
 	/* XXX: move below to kmigrated thread */
 	kmem_cache_destroy(kmigrated_slot_cache);
+	kscand_exit_sysfs(mm_kobj);
 }
 
 void __kscand_enter(struct mm_struct *mm)
@@ -1421,6 +1621,10 @@ static int __init kscand_init(void)
 		return -ENOMEM;
 	}
 
+	err = kscand_init_sysfs(&mm_kobj);
+	if (err)
+		goto err_init_sysfs;
+
 	init_list();
 	err = start_kscand();
 	if (err)
@@ -1437,6 +1641,7 @@ static int __init kscand_init(void)
 
 err_kscand:
 	stop_kscand();
+err_init_sysfs:
 	kscand_destroy();
 
 	return err;
-- 
2.34.1
Re: [RFC PATCH V3 12/17] sysfs: Add sysfs support to tune scanning
Posted by Jonathan Cameron 1 day, 5 hours ago
On Thu, 14 Aug 2025 15:33:02 +0000
Raghavendra K T <raghavendra.kt@amd.com> wrote:

> Support below tunables:
> scan_enable: turn on or turn off mm_struct scanning
> scan_period: initial scan_period (default: 2sec)
> scan_sleep_ms: sleep time between two successive round of scanning and
> migration.
> mms_to_scan: total mm_struct to scan before taking a pause.
> target_node: default regular node to which migration of accessed pages
> is done (this is only fall back mechnism, otherwise target_node
> heuristic is used).
> 
> Signed-off-by: Raghavendra K T <raghavendra.kt@amd.com>

I'd suggest writing
Documentation/ABI/testing/sysfs-...
doc for this as you'll need it in the end and it tends to be easier to review
a doc for this stuff than the code + figuring out where the entrees end up.

Various comments inline.

J
> ---
>  mm/kscand.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
> 
> diff --git a/mm/kscand.c b/mm/kscand.c
> index 41321d373be7..a73606f7ca3c 100644
> --- a/mm/kscand.c
> +++ b/mm/kscand.c
> @@ -21,6 +21,7 @@
>  #include <linux/delay.h>
>  #include <linux/cleanup.h>
>  #include <linux/minmax.h>
> +#include <trace/events/kmem.h>
>  
>  #include <asm/pgalloc.h>
>  #include "internal.h"
> @@ -173,6 +174,171 @@ static bool kscand_eligible_srcnid(int nid)
>  	return  !node_is_toptier(nid);
>  }
>  
> +#ifdef CONFIG_SYSFS

See below. Should not be necessary - the compiler should be able to
see these are unused after it squashes the stubs for sysfs calls
in and hence remove this for us.


> +
> +static struct kobj_attribute scan_sleep_ms_attr =
> +	__ATTR_RW(scan_sleep_ms);

Fits on one line under 80 chars.

> +
> +static ssize_t mm_scan_period_ms_show(struct kobject *kobj,
> +					 struct kobj_attribute *attr,
> +					 char *buf)
> +{
> +	return sysfs_emit(buf, "%u\n", kscand_mm_scan_period_ms);
> +}


> +
> +static struct kobj_attribute mms_to_scan_attr =
> +	__ATTR_RW(mms_to_scan);

Fits on one line.

> +
> +static ssize_t scan_enabled_show(struct kobject *kobj,
> +					 struct kobj_attribute *attr,
> +					 char *buf)

Odd indent.

> +{
> +	return sysfs_emit(buf, "%u\n", kscand_scan_enabled ? 1 : 0);
> +}
> +
> +static ssize_t scan_enabled_store(struct kobject *kobj,
> +					  struct kobj_attribute *attr,
> +					  const char *buf, size_t count)

Another odd looking indent.

> +{
> +	unsigned int val;
> +	int err;
> +
> +	err = kstrtouint(buf, 10, &val);

Maybe use kstrtobool

> +	if (err || val > 1)
> +		return -EINVAL;
> +
> +	if (val) {
> +		kscand_scan_enabled = true;
> +		need_wakeup = true;
> +	} else
> +		kscand_scan_enabled = false;
> +
> +	kscand_sleep_expire = 0;
> +	wake_up_interruptible(&kscand_wait);
> +
> +	return count;
> +}
> +
> +static struct kobj_attribute scan_enabled_attr =
> +	__ATTR_RW(scan_enabled);
One line.

> +

> +}
> +static struct kobj_attribute target_node_attr =
> +	__ATTR_RW(target_node);
One line.
> +
> +static struct attribute *kscand_attr[] = {
> +	&scan_sleep_ms_attr.attr,
> +	&mm_scan_period_ms_attr.attr,
> +	&mms_to_scan_attr.attr,
> +	&scan_enabled_attr.attr,
> +	&target_node_attr.attr,
> +	NULL,

No comma for terminating entries as we don't want to add anything
after this.

> +};
> +
> +struct attribute_group kscand_attr_group = {
> +	.attrs = kscand_attr,
> +	.name = "kscand",
> +};
> +#endif
> +
>  static inline int kscand_has_work(void)
>  {
>  	return !list_empty(&kscand_scan.mm_head);
> @@ -1231,11 +1397,45 @@ static int kscand(void *none)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_SYSFS

The functions used have stubs in sysfs.h so no need
to make these conditional on CONFIG_SYSFS.

Let the compilers dead code removal get rid of the structures
above etc as well.

> +extern struct kobject *mm_kobj;
> +static int __init kscand_init_sysfs(struct kobject **kobj)
> +{
> +	int err;
> +
> +	err = sysfs_create_group(*kobj, &kscand_attr_group);
> +	if (err) {
> +		pr_err("failed to register kscand group\n");
> +		goto err_kscand_attr;
> +	}
> +
> +	return 0;
> +
> +err_kscand_attr:
If create group failed, you shouldn't have anything to remove.



> +	sysfs_remove_group(*kobj, &kscand_attr_group);
> +	return err;
> +}
> +
> +static void __init kscand_exit_sysfs(struct kobject *kobj)
> +{
> +		sysfs_remove_group(kobj, &kscand_attr_group);

Odd indent.

> +}
> +#else
> +static inline int __init kscand_init_sysfs(struct kobject **kobj)
> +{
> +	return 0;
> +}
> +static inline void __init kscand_exit_sysfs(struct kobject *kobj)
> +{
> +}
> +#endif
> +
>  static inline void kscand_destroy(void)
>  {
>  	kmem_cache_destroy(kscand_slot_cache);
>  	/* XXX: move below to kmigrated thread */
>  	kmem_cache_destroy(kmigrated_slot_cache);
> +	kscand_exit_sysfs(mm_kobj);

As it's separate setup step, I'd expect to see this called directly
in error paths in kscand_init() rather than via this helper function.
That way it is easy to see it is only called in paths where it is
appropriate.


>  }
>  
>  void __kscand_enter(struct mm_struct *mm)
> @@ -1421,6 +1621,10 @@ static int __init kscand_init(void)
>  		return -ENOMEM;
>  	}
>  
> +	err = kscand_init_sysfs(&mm_kobj);
> +	if (err)
> +		goto err_init_sysfs;
> +
>  	init_list();
>  	err = start_kscand();
>  	if (err)
> @@ -1437,6 +1641,7 @@ static int __init kscand_init(void)
>  
>  err_kscand:
>  	stop_kscand();
> +err_init_sysfs:
>  	kscand_destroy();
>  
>  	return err;