Re: [RFC PATCH 1/4] STM Ftrace: Adding generic buffer interface driver

[Alexander Shishkin <alexander.shishkin@linux.intel.com>]

Chunyan Zhang <zhang.chunyan@linaro.org> writes:

> This patch adds a driver that models itself as an stm_source and
> who's sole purpose is to export an interface to the rest of the
> kernel.  Once the stm and stm_source have been linked via sysfs,
> everything that is passed to the interface will endup in the STM
> trace engine.

STM core already provides this exact interface to the rest of the
kernel. You need something that ftrace will call into to export its
traces.

>
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
>  drivers/hwtracing/stm/Kconfig      |  9 +++++++
>  drivers/hwtracing/stm/Makefile     |  2 ++
>  drivers/hwtracing/stm/stm_ftrace.c | 54 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>  create mode 100644 drivers/hwtracing/stm/stm_ftrace.c
>
> diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
> index 847a39b..a36633a 100644
> --- a/drivers/hwtracing/stm/Kconfig
> +++ b/drivers/hwtracing/stm/Kconfig
> @@ -39,4 +39,13 @@ config STM_SOURCE_HEARTBEAT
>  	  If you want to send heartbeat messages over STM devices,
>  	  say Y.
>  
> +config STM_FTRACE
> +	tristate "Redirect/copy the output from kernel Ftrace to STM engine"
> +	help
> +	  This option can be used to redirect or copy the output from kernel Ftrace
> +	  to STM engine. Enabling this option will introduce a slight timing effect.

This creates an impression that STM_FTRACE will somehow make events
bypass the normal ftrace ring buffer.

> +
> +	  If you want to send kernel Ftrace messages over STM devices,
> +	  say Y.
> +
>  endif
> diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
> index a9ce3d4..5eef041 100644
> --- a/drivers/hwtracing/stm/Makefile
> +++ b/drivers/hwtracing/stm/Makefile
> @@ -9,3 +9,5 @@ obj-$(CONFIG_STM_SOURCE_HEARTBEAT)	+= stm_heartbeat.o
>  
>  stm_console-y		:= console.o
>  stm_heartbeat-y		:= heartbeat.o
> +
> +obj-$(CONFIG_STM_FTRACE)	+= stm_ftrace.o

We don't have any other source files prefixed with "stm_" in
drivers/hwtracing/stm, because the directory is already "stm".

> diff --git a/drivers/hwtracing/stm/stm_ftrace.c b/drivers/hwtracing/stm/stm_ftrace.c
> new file mode 100644
> index 0000000..0b4eb70
> --- /dev/null
> +++ b/drivers/hwtracing/stm/stm_ftrace.c
> @@ -0,0 +1,54 @@
> +/*
> + * Simple kernel driver to link kernel Ftrace and an STM device
> + * Copyright (c) 2016, Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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/module.h>
> +#include <linux/slab.h>
> +#include <linux/console.h>
> +#include <linux/stm.h>
> +
> +static struct stm_source_data stm_ftrace_data = {
> +	.name		= "stm_ftrace",
> +	.nr_chans	= 1,

Unless you want to allocate one channel per cpu core, which might or
might not be more efficient, but it would be nice to see at least a
comment about it.

> +};
> +
> +/**
> + * stm_ftrace_write() - write data to STM via 'stm_ftrace' source
> + * @buf:	buffer containing the data packet
> + * @len:	length of the data packet
> + * @chan:	offset above the start channel number allocated to 'stm_ftrace'
> + */
> +void notrace stm_ftrace_write(const char *buf, unsigned int len,
> +			      unsigned int chan)
> +{
> +	stm_source_write(&stm_ftrace_data, chan, buf, len);
> +}
> +EXPORT_SYMBOL_GPL(stm_ftrace_write);

An extra wrapper around stm_source_write().

> +
> +static int __init stm_ftrace_init(void)
> +{
> +	return stm_source_register_device(NULL, &stm_ftrace_data);
> +}
> +
> +static void __exit stm_ftrace_exit(void)
> +{
> +	stm_source_unregister_device(&stm_ftrace_data);
> +}

So basically when ftrace is compiled in, it will pull in stm core
through this.

Regards,
--
Alex