xfs
[Top] [All Lists]

Re: [PATCH 1/6] writeback: initial tracing support

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/6] writeback: initial tracing support
From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Date: Thu, 27 May 2010 14:32:33 -0700
Cc: linux-kernel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-ext4@xxxxxxxxxxxxxxx, tytso@xxxxxxx, jens.axboe@xxxxxxxxxx
In-reply-to: <1274784852-30502-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1274784852-30502-1-git-send-email-david@xxxxxxxxxxxxx> <1274784852-30502-2-git-send-email-david@xxxxxxxxxxxxx>
On Tue, 25 May 2010 20:54:07 +1000
Dave Chinner <david@xxxxxxxxxxxxx> wrote:

> From: From: Jens Axboe <jens.axboe@xxxxxxxxxx>
> 
> Trace queue/sched/exec parts of the writeback loop.

It would be most useful if this patchset's description provided sample
tracing output, so we can see what the patch is actually providing us.

> -#define inode_to_bdi(inode)  ((inode)->i_mapping->backing_dev_info)
> -
> -/*
> - * We don't actually have pdflush, but this one is exported though /proc...
> - */
> -int nr_pdflush_threads;
> -
>  /*
>   * Passed into wb_writeback(), essentially a subset of writeback_control
>   */
> @@ -63,6 +57,16 @@ struct bdi_work {
>       unsigned long state;            /* flag bits, see WS_* */
>  };
>  
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/writeback.h>
> +
> +#define inode_to_bdi(inode)  ((inode)->i_mapping->backing_dev_info)

Could/should be implemented in C.

> +/*
> + * We don't actually have pdflush, but this one is exported though /proc...
> + */
> +int nr_pdflush_threads;

So this is always zero now?

We don't want to keep it forever.  Add a
printk_once("nr_pdflush_threads is deprecated") when someone reads it,
remove it in 2014.

>
> ...
>
> --- /dev/null
> +++ b/include/trace/events/writeback.h
> @@ -0,0 +1,171 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM writeback
> +
> +#if !defined(_TRACE_WRITEBACK_H) || defined(TRACE_HEADER_MULTI_READ)

Seems wrong.  If you define TRACE_HEADER_MULTI_READ then include this
header twice, things explode.  Which negates the purpose of
_TRACE_WRITEBACK_H.

> +#define _TRACE_WRITEBACK_H
> +
> +#include <linux/backing-dev.h>
> +#include <linux/writeback.h>
> +
> +TRACE_EVENT(writeback_queue,
> +
> +     TP_PROTO(struct backing_dev_info *bdi, struct wb_writeback_args *args),
> +
> +     TP_ARGS(bdi, args),
> +
> +     TP_STRUCT__entry(
> +             __array(char,           name,           16)
> +             __field(long,           nr_pages)
> +             __field(int,            sb)
> +             __field(int,            sync_mode)
> +             __field(int,            for_kupdate)
> +             __field(int,            range_cyclic)
> +             __field(int,            for_background)
> +     ),
> +
> +     TP_fast_assign(
> +             strncpy(__entry->name, dev_name(bdi->dev), 16);
> +             __entry->nr_pages       = args->nr_pages;
> +             __entry->sb             = !!args->sb;
> +             __entry->for_kupdate    = args->for_kupdate;
> +             __entry->range_cyclic   = args->range_cyclic;
> +             __entry->for_background = args->for_background;
> +     ),
> +
> +     TP_printk("%s: pages=%ld, sb=%d, kupdate=%d, range_cyclic=%d "
> +               "for_background=%d", __entry->name, __entry->nr_pages,
> +                     __entry->sb, __entry->for_kupdate,
> +                     __entry->range_cyclic, __entry->for_background)
> +);
> +

<Prev in Thread] Current Thread [Next in Thread>