Re: [RFC PATCH V3 02/17] mm: Maintain mm_struct list in the system

Jonathan Cameron
2025-10-02T13:23:21Z

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

> The list is used to iterate over all the mm and do PTE A bit scanning.
> mm_slot infrastructure is reused to aid insert and lookup of mm_struct.
>
> CC: linux-fsdevel@vger.kernel.org

This is part of the tags block. Some tools moan if you have blank lines
in that. Alternatively push it below the --- and it won't end up in
the git commit but tooling will still add the +CC.

>
> Suggested-by: Bharata B Rao <bharata@amd.com>
> Signed-off-by: Raghavendra K T <raghavendra.kt@amd.com>

A few minor comments on formatting.

> diff --git a/mm/kscand.c b/mm/kscand.c
> index f7bbbc70c86a..d5b0d3041b0f 100644
> --- a/mm/kscand.c
> +++ b/mm/kscand.c

> +void __kscand_enter(struct mm_struct *mm)
> +{
> + struct kscand_mm_slot *kscand_slot;
> + struct mm_slot *slot;
> + int wakeup;
> +
> + /* __kscand_exit() must not run from under us */
> + VM_BUG_ON_MM(kscand_test_exit(mm), mm);
> +
> + kscand_slot = mm_slot_alloc(kscand_slot_cache);
> +
Similar to below. I'd keep call and error check more closely coupled
visually by dropping this blank line.

> + if (!kscand_slot)
> + return;
> +
> + slot = &kscand_slot->slot;
> +
> + spin_lock(&kscand_mm_lock);
> + mm_slot_insert(kscand_slots_hash, mm, slot);
> +
> + wakeup = list_empty(&kscand_scan.mm_head);

Looks familiar.

wakeup = kscand_has_work()

Or maybe just get rid of that helper and check it explicitly like this.

> + list_add_tail(&slot->mm_node, &kscand_scan.mm_head);
> + spin_unlock(&kscand_mm_lock);
> +
> + mmgrab(mm);
> + if (wakeup)
> + wake_up_interruptible(&kscand_wait);
> +}

> +
> static int start_kscand(void)
> {
> struct task_struct *kthread;
> @@ -149,6 +228,12 @@ static int __init kscand_init(void)
> {
> int err;
>
> + kscand_slot_cache = KMEM_CACHE(kscand_mm_slot, 0);
> +

I'd drop this blank line. Keep the call and the error check tightly coupled
in one block of code.

> + if (!kscand_slot_cache) {
> + pr_err("kscand: kmem_cache error");
> + return -ENOMEM;
> + }
> err = start_kscand();
> if (err)
> goto err_kscand;
> @@ -157,6 +242,7 @@ static int __init kscand_init(void)
>
> err_kscand:
> stop_kscand();
> + kscand_destroy();
>
> return err;
> }