Re: [PATCH v9 04/27] x86/resctrl: resctrl_exit() teardown resctrl but leave the mount point

[Reinette Chatre]

Hi James,

On 4/25/25 10:37 AM, James Morse wrote:
> @@ -4432,23 +4445,42 @@ static bool __exit resctrl_online_domains_exist(void)
>  	return false;
>  }
>  
> -/*
> +/**

Why make the switch to kernel-doc now? The benefit is not clear considering
resctrl_init() is not using kernel-doc.

>   * resctrl_exit() - Remove the resctrl filesystem and free resources.
>   *
> + * Called by the architecture code in response to a fatal error.
> + * Removes resctrl files and structures from kernfs to prevent further
> + * configuration.
> + *
>   * When called by the architecture code, all CPUs and resctrl domains must be
>   * offline. This ensures the limbo and overflow handlers are not scheduled to
>   * run, meaning the data structures they access can be freed by
>   * resctrl_mon_resource_exit().
> + *
> + * After this function has returned, the architecture code should return an

nit: "After this function has returned," -> "After resctrl_exit() returns, "

"should return an" -> "should return an error"?

> + * from all resctrl_arch_ functions that can do this.
> + * resctrl_arch_get_resource() must continue to return struct rdt_resources
> + * with the correct rid field to ensure the filesystem can be unmounted.

Is this to get through set_mba_sc() and the for_each_alloc_capable_rdt_resource(r)
loop in rdt_kill_sb() or is there something more subtle?

>   */
>  void __exit resctrl_exit(void)
>  {
>  	cpus_read_lock();
>  	WARN_ON_ONCE(resctrl_online_domains_exist());
> +
> +	mutex_lock(&rdtgroup_mutex);
> +	resctrl_fs_teardown();
> +	mutex_unlock(&rdtgroup_mutex);
> +
>  	cpus_read_unlock();
>  
>  	debugfs_remove_recursive(debugfs_resctrl);
> +	debugfs_resctrl = NULL;
>  	unregister_filesystem(&rdt_fs_type);
> -	sysfs_remove_mount_point(fs_kobj, "resctrl");
> +
> +	/*
> +	 * Do not remove the sysfs mount point added by resctrl_init() so that
> +	 * it can be used to umount resctrl.
> +	 */
>  
>  	resctrl_mon_resource_exit();
>  }

Looks good to me.

Thank you.

Reinette