Re: [patch] drop_caches: add some documentation and info message

[Johannes Weiner]

On Fri, Feb 07, 2014 at 12:31:29PM -0800, Andrew Morton wrote:
> On Fri, 7 Feb 2014 13:13:32 -0500 Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
> 
> > @@ -63,6 +64,9 @@ int drop_caches_sysctl_handler(ctl_table *table, int write,
> >  			iterate_supers(drop_pagecache_sb, NULL);
> >  		if (sysctl_drop_caches & 2)
> >  			drop_slab();
> > +		printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> > +				   current->comm, task_pid_nr(current),
> > +				   sysctl_drop_caches);
> >  	}
> >  	return 0;
> >  }
> 
> My concern with this is that there may be people whose
> other-party-provided software uses drop_caches.  Their machines will
> now sit there emitting log messages and there's nothing they can do
> about it, apart from whining at their vendors.

Ironically, we have a customer that is complaining that we currently
do not log these events, and they want to know who in their stack is
being idiotic.

> We could do something like this?

They can already change the log level.  The below will suppress
valuable debugging information in a way that still results in
inconspicuous looking syslog excerpts, which somewhat undermines the
original motivation for this change.

So I'm not fond of it, but I'd rather have this patch with it than no
patch at all.  As long as the message is printed per default.

> --- a/fs/drop_caches.c~drop_caches-add-some-documentation-and-info-message-fix
> +++ a/fs/drop_caches.c
> @@ -60,13 +60,17 @@ int drop_caches_sysctl_handler(ctl_table
>  	if (ret)
>  		return ret;
>  	if (write) {
> +		static int stfu;
> +
>  		if (sysctl_drop_caches & 1)
>  			iterate_supers(drop_pagecache_sb, NULL);
>  		if (sysctl_drop_caches & 2)
>  			drop_slab();
> -		printk_ratelimited(KERN_INFO "%s (%d): dropped kernel caches: %d\n",
> -				   current->comm, task_pid_nr(current),
> -				   sysctl_drop_caches);
> +		stfu |= sysctl_drop_caches & 4;
> +		if (!stfu)
> +			pr_info_ratelimited("%s (%d): dropped kernel caches: %d\n",
> +					   current->comm, task_pid_nr(current),
> +					   sysctl_drop_caches);
>  	}
>  	return 0;
>  }
> _
> 
> (note switch to pr_info_ratelimited)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/