Re: [PATCH] mm/kfence: print disabling or re-enabling message

[Marco Elver <elver@google.com>]

On Tue, May 17, 2022 at 07:15PM +0800, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> By printing information, we can friendly prompt the status change
> information of kfence by dmesg.
> 
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>

Personally, I've never found this useful. If I want to get the current
accurate state of KFENCE enablement, I just look at
/sys/kernel/debug/kfence/stats.

Nevertheless, some comments below.

> ---
>  mm/kfence/core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 11a954763be9..beb552089b67 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -67,8 +67,11 @@ static int param_set_sample_interval(const char *val, const struct kernel_param
>  	if (ret < 0)
>  		return ret;
>  
> -	if (!num) /* Using 0 to indicate KFENCE is disabled. */
> +	if (!num) {
> +		/* Using 0 to indicate KFENCE is disabled. */
>  		WRITE_ONCE(kfence_enabled, false);
> +		pr_info("KFENCE is disabled.\n");

This will also print on boot if kfence.sample_interval=0 is passed. This
is ugly.

We also have a pr_fmt, and writing "KFENCE" again is ugly, too. And
adding '.' at the end of these short log lines is not something done
much in the kernel, and also ugly.

So what you want is this fixup:
 
diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index beb552089b67..de5bcf2609fe 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -67,10 +67,11 @@ static int param_set_sample_interval(const char *val, const struct kernel_param
 	if (ret < 0)
 		return ret;
 
+	/* Using 0 to indicate KFENCE is disabled. */
 	if (!num) {
-		/* Using 0 to indicate KFENCE is disabled. */
+		if (READ_ONCE(kfence_enabled))
+			pr_info("disabled\n");
 		WRITE_ONCE(kfence_enabled, false);
-		pr_info("KFENCE is disabled.\n");
 	}
 
 	*((unsigned long *)kp->arg) = num;
@@ -877,7 +878,7 @@ static int kfence_enable_late(void)
 
 	WRITE_ONCE(kfence_enabled, true);
 	queue_delayed_work(system_unbound_wq, &kfence_timer, 0);
-	pr_info("KFENCE is re-enabled.\n");
+	pr_info("re-enabled\n");
 	return 0;
 }