Re: [PATCH 1/5] ftrace: limit use of check pages

[Pekka Paalanen]

On Thu, 22 May 2008 00:22:16 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> The check_pages function is called often enough that it can cause problems
> with trace outputs or even bringing the system to a halt.
> 
> This patch limits the check_pages to the places that are most likely to
> have problems. The check is made at the flip between the global array and
> the max save array, as well as when the size of the buffers changes and
> the self tests.
> 
> This patch also removes the BUG_ON from check_pages and replaces it with
> a WARN_ON and disabling of the tracer.
> 
> Signed-off-by: Steven Rostedt <srostedt@xxxxxxxxxx>
> ---
>  kernel/trace/trace.c          |   32 +++++++++++++++++++++++---------
>  kernel/trace/trace_selftest.c |    1 +
>  2 files changed, 24 insertions(+), 9 deletions(-)
> 
> Index: linux-tip.git/kernel/trace/trace.c
> ===================================================================
> --- linux-tip.git.orig/kernel/trace/trace.c	2008-05-21 13:25:34.000000000 -0400
> +++ linux-tip.git/kernel/trace/trace.c	2008-05-21 21:32:23.000000000 -0400
> @@ -249,24 +249,32 @@ __update_max_tr(struct trace_array *tr, 
>  	tracing_record_cmdline(current);
>  }
>  
> +#define CHECK_COND(cond)			\
> +	if (unlikely(cond)) {			\
> +		tracing_disabled = 0;		\

-----------------------------------^
This looks like a typo.

I also quickly read through the other patches and didn't see any
obvious problems in them.

> +		WARN_ON(1);			\
> +		return -1;			\
> +	}
> +
>  /**
>   * check_pages - integrity check of trace buffers
>   *
>   * As a safty measure we check to make sure the data pages have not
> - * been corrupted. TODO: configure to disable this because it adds
> - * a bit of overhead.
> + * been corrupted.
>   */
> -void check_pages(struct trace_array_cpu *data)
> +int check_pages(struct trace_array_cpu *data)
>  {
>  	struct page *page, *tmp;
>  
> -	BUG_ON(data->trace_pages.next->prev != &data->trace_pages);
> -	BUG_ON(data->trace_pages.prev->next != &data->trace_pages);
> +	CHECK_COND(data->trace_pages.next->prev != &data->trace_pages);
> +	CHECK_COND(data->trace_pages.prev->next != &data->trace_pages);
>  
>  	list_for_each_entry_safe(page, tmp, &data->trace_pages, lru) {
> -		BUG_ON(page->lru.next->prev != &page->lru);
> -		BUG_ON(page->lru.prev->next != &page->lru);
> +		CHECK_COND(page->lru.next->prev != &page->lru);
> +		CHECK_COND(page->lru.prev->next != &page->lru);
>  	}
> +
> +	return 0;
>  }


-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
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/