[PATCH v2] drop_caches: Allow re-enabling message after disabling

[PATCH v2] drop_caches: Allow re-enabling message after disabling

[Joel Granados]

After writing "4" to /proc/sys/vm/drop_caches there was no way to
re-enable the drop_caches kernel message. By replacing the "or"
assignment for the stfu variable, it is now possible to toggle the
message on and off by setting the 4th bit in /proc/sys/vm/drop_caches.

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
Changes in v2:
- Check the 4 bit before we actualy toggle the message
- Link to v1: https://lore.kernel.org/r/20250313-jag-drop_caches_msg-v1-1-c2e4e7874b72@kernel.org
---

---
 Documentation/admin-guide/sysctl/vm.rst | 2 +-
 fs/drop_caches.c                        | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
index f48eaa98d22d2b575f6e913f437b0d548daac3e6..75a032f8cbfb4e05f04610cca219d154bd852789 100644
--- a/Documentation/admin-guide/sysctl/vm.rst
+++ b/Documentation/admin-guide/sysctl/vm.rst
@@ -266,7 +266,7 @@ used::
 	cat (1234): drop_caches: 3
 
 These are informational only.  They do not mean that anything is wrong
-with your system.  To disable them, echo 4 (bit 2) into drop_caches.
+with your system.  To toggle them, echo 4 (bit 2) into drop_caches.
 
 enable_soft_offline
 ===================
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index d45ef541d848a73cbd19205e0111c2cab3b73617..15730593ae39955ae7ae93aec17546fc96f89dce 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -68,12 +68,13 @@ int drop_caches_sysctl_handler(const struct ctl_table *table, int write,
 			drop_slab();
 			count_vm_event(DROP_SLAB);
 		}
+		if (sysctl_drop_caches & 4)
+			stfu ^= 1;
 		if (!stfu) {
 			pr_info("%s (%d): drop_caches: %d\n",
 				current->comm, task_pid_nr(current),
 				sysctl_drop_caches);
 		}
-		stfu |= sysctl_drop_caches & 4;
 	}
 	return 0;
 }

---
base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
change-id: 20250313-jag-drop_caches_msg-c4fbfedb51f3

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>

Re: [PATCH v2] drop_caches: Allow re-enabling message after disabling

[Matthew Wilcox]

On Mon, Mar 17, 2025 at 08:40:04PM +0100, Joel Granados wrote:
> After writing "4" to /proc/sys/vm/drop_caches there was no way to
> re-enable the drop_caches kernel message. By replacing the "or"
> assignment for the stfu variable, it is now possible to toggle the
> message on and off by setting the 4th bit in /proc/sys/vm/drop_caches.

I don't like the toggle.  Nobody wants to toggle, which means that you
need to keep track of what it is in order to make it be "on" or "off".
And you can't keep track of it, because it's system-wide.  Which means
you might turn it off when you wanted it on, or vice versa.

Did I miss the discussion which promopted this change?  It seems like
terrible UI to me.

> Signed-off-by: Joel Granados <joel.granados@kernel.org>
> ---
> Changes in v2:
> - Check the 4 bit before we actualy toggle the message
> - Link to v1: https://lore.kernel.org/r/20250313-jag-drop_caches_msg-v1-1-c2e4e7874b72@kernel.org
> ---
> 
> ---
>  Documentation/admin-guide/sysctl/vm.rst | 2 +-
>  fs/drop_caches.c                        | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
> index f48eaa98d22d2b575f6e913f437b0d548daac3e6..75a032f8cbfb4e05f04610cca219d154bd852789 100644
> --- a/Documentation/admin-guide/sysctl/vm.rst
> +++ b/Documentation/admin-guide/sysctl/vm.rst
> @@ -266,7 +266,7 @@ used::
>  	cat (1234): drop_caches: 3
>  
>  These are informational only.  They do not mean that anything is wrong
> -with your system.  To disable them, echo 4 (bit 2) into drop_caches.
> +with your system.  To toggle them, echo 4 (bit 2) into drop_caches.
>  
>  enable_soft_offline
>  ===================
> diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> index d45ef541d848a73cbd19205e0111c2cab3b73617..15730593ae39955ae7ae93aec17546fc96f89dce 100644
> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -68,12 +68,13 @@ int drop_caches_sysctl_handler(const struct ctl_table *table, int write,
>  			drop_slab();
>  			count_vm_event(DROP_SLAB);
>  		}
> +		if (sysctl_drop_caches & 4)
> +			stfu ^= 1;
>  		if (!stfu) {
>  			pr_info("%s (%d): drop_caches: %d\n",
>  				current->comm, task_pid_nr(current),
>  				sysctl_drop_caches);
>  		}
> -		stfu |= sysctl_drop_caches & 4;
>  	}
>  	return 0;
>  }
> 
> ---
> base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
> change-id: 20250313-jag-drop_caches_msg-c4fbfedb51f3
> 
> Best regards,
> -- 
> Joel Granados <joel.granados@kernel.org>
> 
> 
> 

Re: [PATCH v2] drop_caches: Allow re-enabling message after disabling

[Joel Granados]

On Mon, Mar 17, 2025 at 07:53:36PM +0000, Matthew Wilcox wrote:
> On Mon, Mar 17, 2025 at 08:40:04PM +0100, Joel Granados wrote:
> > After writing "4" to /proc/sys/vm/drop_caches there was no way to
> > re-enable the drop_caches kernel message. By replacing the "or"
> > assignment for the stfu variable, it is now possible to toggle the
> > message on and off by setting the 4th bit in /proc/sys/vm/drop_caches.
> 
> I don't like the toggle.  Nobody wants to toggle, which means that you
> need to keep track of what it is in order to make it be "on" or "off".
> And you can't keep track of it, because it's system-wide.  Which means
> you might turn it off when you wanted it on, or vice versa.
Don't really have a strong opinion here. Posted V2 thinking that
toggling might be preferable. If it is not, then V1 is the way
to go as it just sets the value by looking at the 4th bit.

> 
> Did I miss the discussion which promopted this change?  It seems like
> terrible UI to me.

As I understand it, the motivation is to be able to turn the messages
back on. This is not possible as the 4th bit stays set (no message)
after setting it for the first time.

Best
> 
> > Signed-off-by: Joel Granados <joel.granados@kernel.org>
> > ---
> > Changes in v2:
> > - Check the 4 bit before we actualy toggle the message
> > - Link to v1: https://lore.kernel.org/r/20250313-jag-drop_caches_msg-v1-1-c2e4e7874b72@kernel.org
> > ---
> > 
> > ---
> >  Documentation/admin-guide/sysctl/vm.rst | 2 +-
> >  fs/drop_caches.c                        | 3 ++-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/sysctl/vm.rst b/Documentation/admin-guide/sysctl/vm.rst
> > index f48eaa98d22d2b575f6e913f437b0d548daac3e6..75a032f8cbfb4e05f04610cca219d154bd852789 100644
> > --- a/Documentation/admin-guide/sysctl/vm.rst
> > +++ b/Documentation/admin-guide/sysctl/vm.rst
> > @@ -266,7 +266,7 @@ used::
> >  	cat (1234): drop_caches: 3
> >  
> >  These are informational only.  They do not mean that anything is wrong
> > -with your system.  To disable them, echo 4 (bit 2) into drop_caches.
> > +with your system.  To toggle them, echo 4 (bit 2) into drop_caches.
> >  
> >  enable_soft_offline
> >  ===================
> > diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> > index d45ef541d848a73cbd19205e0111c2cab3b73617..15730593ae39955ae7ae93aec17546fc96f89dce 100644
> > --- a/fs/drop_caches.c
> > +++ b/fs/drop_caches.c
> > @@ -68,12 +68,13 @@ int drop_caches_sysctl_handler(const struct ctl_table *table, int write,
> >  			drop_slab();
> >  			count_vm_event(DROP_SLAB);
> >  		}
> > +		if (sysctl_drop_caches & 4)
> > +			stfu ^= 1;
> >  		if (!stfu) {
> >  			pr_info("%s (%d): drop_caches: %d\n",
> >  				current->comm, task_pid_nr(current),
> >  				sysctl_drop_caches);
> >  		}
> > -		stfu |= sysctl_drop_caches & 4;
> >  	}
> >  	return 0;
> >  }
> > 
> > ---
> > base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6
> > change-id: 20250313-jag-drop_caches_msg-c4fbfedb51f3
> > 
> > Best regards,
> > -- 
> > Joel Granados <joel.granados@kernel.org>
> > 
> > 
> > 

-- 

Joel Granados