Re: [PATCH 3/8] pstore: Add persistent function tracing

[Steven Rostedt]

On Mon, 2012-07-09 at 17:10 -0700, Anton Vorontsov wrote:

> --- /dev/null
> +++ b/fs/pstore/ftrace.c
> @@ -0,0 +1,35 @@
> +/*
> + * Copyright 2012  Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/compiler.h>
> +#include <linux/irqflags.h>
> +#include <linux/percpu.h>
> +#include <linux/smp.h>
> +#include <linux/atomic.h>
> +#include <asm/barrier.h>
> +#include "internal.h"
> +
> +void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip)

BTW, you can make the entire file 'notrace' without adding annotations
by including in the Makefile:

CFLAGS_REMOVE_ftrace.o = -pg


> +{
> +	struct pstore_ftrace_record rec = {};
> +
> +	if (unlikely(oops_in_progress))
> +		return;
> +
> +	rec.ip = ip;
> +	rec.parent_ip = parent_ip;
> +	pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id());
> +	psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec,
> +			  sizeof(rec), psinfo);
> +}

BTW, can any of the called functions go into module code that can be
removed? If so, then this is not safe at all. Normal function tracing
can not be synced in a preemptible kernel.

Also, I'm starting to wonder if this should be in its own utility
(separate debugfs?) than hooking directly into ftrace. Then you don't
need to modify ftrace at all and you can do the following:

static struct ftrace_ops trace_ops {
	.func = pstore_ftrace_call;
};

then in your write to debugfs file:

	register_ftrace_function(&trace_ops);

To turn off tracing:

	unregister_ftrace_function(&trace_ops);

Note, it's safe to use if the trace_ops (or anything the callback calls)
is a module. That's because it detects the trace_ops is not kernel core
code and will place a wrapper around it that allows the function tracing
to by synced with module unload. You still need to unregister the
trace_ops before unloading the module, or you can have a crash that way.

-- Steve


--
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/