Re: [PATCH] writeback: mark sysctl vm.block_dump for removal

[Jan Kara <jack@suse.cz>]

On Mon 14-01-13 10:22:03, Tejun Heo wrote:
> vm.block_dump is nasty in that it dumps IO traces directly to printk.
> It isn't scalable and can easily lead to looping behavior - IO
> generating kernel message which in turn genreates log IO, ad
> infinitum.
> 
> Now that proper tracepoints are in place, vm.block_dump isn't
> necessary.  Generate a warning if used so that it can be removed down
> the road.
  Looks good. Maybe we could extend the message to ask people to let us know
if they see the message? If someone is using it we shouldn't really remove
it.

								Honza
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  include/linux/writeback.h |    3 +++
>  kernel/sysctl.c           |    2 +-
>  mm/page-writeback.c       |   14 ++++++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
> index b82a83a..678ee6d 100644
> --- a/include/linux/writeback.h
> +++ b/include/linux/writeback.h
> @@ -143,6 +143,9 @@ extern int dirty_ratio_handler(struct ctl_table *table, int write,
>  extern int dirty_bytes_handler(struct ctl_table *table, int write,
>  		void __user *buffer, size_t *lenp,
>  		loff_t *ppos);
> +extern int block_dump_handler(struct ctl_table *table, int write,
> +		void __user *buffer, size_t *lenp,
> +		loff_t *ppos);
>  
>  struct ctl_table;
>  int dirty_writeback_centisecs_handler(struct ctl_table *, int,
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index c88878d..9e7f7b5 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1288,7 +1288,7 @@ static struct ctl_table vm_table[] = {
>  		.data		= &block_dump,
>  		.maxlen		= sizeof(block_dump),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= block_dump_handler,
>  		.extra1		= &zero,
>  	},
>  	{
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 6f42712..5548bef 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -390,6 +390,20 @@ int dirty_bytes_handler(struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> +int block_dump_handler(struct ctl_table *table, int write,
> +		       void __user *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	static bool warned = false;
> +	int ret;
> +
> +	ret = proc_dointvec(table, write, buffer, lenp, ppos);
> +	if (!warned && block_dump) {
> +		pr_warning("sysctl: vm.block_dump is scheduled for removal. Please use tracepoints instead.\n");
> +		warned = true;
> +	}
> +	return ret;
> +}
> +
>  static unsigned long wp_next_time(unsigned long cur_time)
>  {
>  	cur_time += VM_COMPLETIONS_PERIOD_LEN;
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR