Re: [RFC PATCH V3 05/17] mm: Create a separate kthread for migration

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Thu, 14 Aug 2025 15:32:55 +0000
Raghavendra K T <raghavendra.kt@amd.com> wrote:

> Having independent thread helps in:
>  - Alleviating the need for multiple scanning threads
>  - Aids to control batch migration (TBD)
>  - Migration throttling (TBD)
> 
> Signed-off-by: Raghavendra K T <raghavendra.kt@amd.com>
A few superficial things inline.

> ---
>  mm/kscand.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/mm/kscand.c b/mm/kscand.c
> index 7552ce32beea..55efd0a6e5ba 100644
> --- a/mm/kscand.c
> +++ b/mm/kscand.c
> @@ -4,6 +4,7 @@
>  #include <linux/sched.h>
>  #include <linux/sched/mm.h>
>  #include <linux/mmu_notifier.h>
> +#include <linux/migrate.h>
>  #include <linux/rmap.h>
>  #include <linux/pagewalk.h>
>  #include <linux/page_ext.h>
> @@ -41,6 +42,15 @@ static unsigned long kscand_mms_to_scan __read_mostly = KSCAND_MMS_TO_SCAN;
>  
>  bool kscand_scan_enabled = true;
>  static bool need_wakeup;
> +static bool migrated_need_wakeup;
> +
> +/* How long to pause between two migration cycles */
> +static unsigned int kmigrate_sleep_ms __read_mostly = 20;
> +
> +static struct task_struct *kmigrated_thread __read_mostly;
> +static DEFINE_MUTEX(kmigrated_mutex);
> +static DECLARE_WAIT_QUEUE_HEAD(kmigrated_wait);
> +static unsigned long kmigrated_sleep_expire;
>  
>  static unsigned long kscand_sleep_expire;
>  
> @@ -79,6 +89,7 @@ struct kscand_scanctrl {
>  };
>  
>  struct kscand_scanctrl kscand_scanctrl;
> +
Unrelated change. If you want it make sure to push back to earlier patch.


>  /* Per folio information used for migration */
>  struct kscand_migrate_info {
>  	struct list_head migrate_node;
> @@ -134,6 +145,19 @@ static inline bool is_valid_folio(struct folio *folio)
>  	return true;
>  }
>  
> +static inline void kmigrated_wait_work(void)
> +{
> +	const unsigned long migrate_sleep_jiffies =
> +		msecs_to_jiffies(kmigrate_sleep_ms);
> +
> +	if (!migrate_sleep_jiffies)
> +		return;
> +
> +	kmigrated_sleep_expire = jiffies + migrate_sleep_jiffies;
> +	wait_event_timeout(kmigrated_wait,
> +			true,
> +			migrate_sleep_jiffies);

Unusual line wrapping.  I'd go nearer to 80 chars and put at least
some paramters on same line.

> +}

>  static inline void init_list(void)
>  {
>  	INIT_LIST_HEAD(&kscand_scanctrl.scan_list);
> @@ -559,8 +626,15 @@ static int __init kscand_init(void)
>  	if (err)
>  		goto err_kscand;
>  
> +	err = start_kmigrated();
> +	if (err)
> +		goto err_kmigrated;

Similar comment to earlier on not expecting side effects of a thread to stop
if the start failed.

> +
>  	return 0;
>  
> +err_kmigrated:
> +	stop_kmigrated();
> +
>  err_kscand:
>  	stop_kscand();
>  	kscand_destroy();