[PATCH] kernel: trace: pass export pointer as argument to ->write()

[PATCH] kernel: trace: pass export pointer as argument to ->write()

[Felipe Balbi]

That way, users don't need to keep a global static pointer and can
rely on container_of() to fetch their own structure.

Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---

I'm working on exposing this data over a USB peripheral controller and
I really can't have a global pointer there.

 drivers/hwtracing/stm/ftrace.c | 6 ++++--
 include/linux/trace.h          | 2 +-
 kernel/trace/trace.c           | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
index bd126a7c6da2..371b8333d9ae 100644
--- a/drivers/hwtracing/stm/ftrace.c
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -42,9 +42,11 @@ static struct stm_ftrace {
  * @len:	length of the data packet
  */
 static void notrace
-stm_ftrace_write(const void *buf, unsigned int len)
+stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
 {
-	stm_source_write(&stm_ftrace.data, STM_FTRACE_CHAN, buf, len);
+	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
+
+	stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
 }
 
 static int stm_ftrace_link(struct stm_source_data *data)
diff --git a/include/linux/trace.h b/include/linux/trace.h
index 9330a58e2651..477aed6f0caa 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -17,7 +17,7 @@
  */
 struct trace_export {
 	struct trace_export __rcu	*next;
-	void (*write)(const void *, unsigned int);
+	void (*write)(struct trace_export *, const void *, unsigned int);
 };
 
 int register_ftrace_export(struct trace_export *export);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c4536c449021..6a96652a3b0f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2317,7 +2317,7 @@ trace_process_export(struct trace_export *export,
 
 	entry = ring_buffer_event_data(event);
 	size = ring_buffer_event_length(event);
-	export->write(entry, size);
+	export->write(export, entry, size);
 }
 
 static DEFINE_MUTEX(ftrace_export_lock);
-- 
2.11.0.295.gd7dffce1ce

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Steven Rostedt]

On Wed, 17 May 2017 11:05:54 +0300
Felipe Balbi <felipe.balbi@linux.intel.com> wrote:

> That way, users don't need to keep a global static pointer and can
> rely on container_of() to fetch their own structure.
> 
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> ---
> 
> I'm working on exposing this data over a USB peripheral controller and
> I really can't have a global pointer there.
> 
>  drivers/hwtracing/stm/ftrace.c | 6 ++++--
>  include/linux/trace.h          | 2 +-
>  kernel/trace/trace.c           | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
> index bd126a7c6da2..371b8333d9ae 100644
> --- a/drivers/hwtracing/stm/ftrace.c
> +++ b/drivers/hwtracing/stm/ftrace.c
> @@ -42,9 +42,11 @@ static struct stm_ftrace {
>   * @len:	length of the data packet
>   */
>  static void notrace
> -stm_ftrace_write(const void *buf, unsigned int len)
> +stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
>  {
> -	stm_source_write(&stm_ftrace.data, STM_FTRACE_CHAN, buf, len);
> +	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
> +
> +	stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
>  }
>  
>  static int stm_ftrace_link(struct stm_source_data *data)
> diff --git a/include/linux/trace.h b/include/linux/trace.h
> index 9330a58e2651..477aed6f0caa 100644
> --- a/include/linux/trace.h
> +++ b/include/linux/trace.h
> @@ -17,7 +17,7 @@
>   */
>  struct trace_export {
>  	struct trace_export __rcu	*next;
> -	void (*write)(const void *, unsigned int);
> +	void (*write)(struct trace_export *, const void *, unsigned int);
>  };
>  
>  int register_ftrace_export(struct trace_export *export);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c4536c449021..6a96652a3b0f 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2317,7 +2317,7 @@ trace_process_export(struct trace_export *export,
>  
>  	entry = ring_buffer_event_data(event);
>  	size = ring_buffer_event_length(event);
> -	export->write(entry, size);
> +	export->write(export, entry, size);
>  }
>  
>  static DEFINE_MUTEX(ftrace_export_lock);

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Chunyan Zhang]

On 17 May 2017 at 16:05, Felipe Balbi <felipe.balbi@linux.intel.com> wrote:
> That way, users don't need to keep a global static pointer and can
> rely on container_of() to fetch their own structure.
>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>

Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>

Thanks,
Chunyan

> ---
>
> I'm working on exposing this data over a USB peripheral controller and
> I really can't have a global pointer there.
>
>  drivers/hwtracing/stm/ftrace.c | 6 ++++--
>  include/linux/trace.h          | 2 +-
>  kernel/trace/trace.c           | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
> index bd126a7c6da2..371b8333d9ae 100644
> --- a/drivers/hwtracing/stm/ftrace.c
> +++ b/drivers/hwtracing/stm/ftrace.c
> @@ -42,9 +42,11 @@ static struct stm_ftrace {
>   * @len:       length of the data packet
>   */
>  static void notrace
> -stm_ftrace_write(const void *buf, unsigned int len)
> +stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
>  {
> -       stm_source_write(&stm_ftrace.data, STM_FTRACE_CHAN, buf, len);
> +       struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
> +
> +       stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
>  }
>
>  static int stm_ftrace_link(struct stm_source_data *data)
> diff --git a/include/linux/trace.h b/include/linux/trace.h
> index 9330a58e2651..477aed6f0caa 100644
> --- a/include/linux/trace.h
> +++ b/include/linux/trace.h
> @@ -17,7 +17,7 @@
>   */
>  struct trace_export {
>         struct trace_export __rcu       *next;
> -       void (*write)(const void *, unsigned int);
> +       void (*write)(struct trace_export *, const void *, unsigned int);
>  };
>
>  int register_ftrace_export(struct trace_export *export);
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index c4536c449021..6a96652a3b0f 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2317,7 +2317,7 @@ trace_process_export(struct trace_export *export,
>
>         entry = ring_buffer_event_data(event);
>         size = ring_buffer_event_length(event);
> -       export->write(entry, size);
> +       export->write(export, entry, size);
>  }
>
>  static DEFINE_MUTEX(ftrace_export_lock);
> --
> 2.11.0.295.gd7dffce1ce
>

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Steven Rostedt]

On Thu, 18 May 2017 10:26:59 +0800
Chunyan Zhang <zhang.chunyan@linaro.org> wrote:

> On 17 May 2017 at 16:05, Felipe Balbi <felipe.balbi@linux.intel.com> wrote:
> > That way, users don't need to keep a global static pointer and can
> > rely on container_of() to fetch their own structure.
> >
> > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>  
> 
> Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>


If someone wants to pull this through their tree, feel free to do
so. I've already acked it.

-- Steve

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1710 bytes --]

Hi Felipe,

[auto build test ERROR on tip/perf/core]
[also build test ERROR on v4.12-rc1 next-20170518]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Felipe-Balbi/kernel-trace-pass-export-pointer-as-argument-to-write/20170518-135452
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/hwtracing/stm/ftrace.c: In function 'stm_ftrace_write':
>> drivers/hwtracing/stm/ftrace.c:49:19: error: incompatible type for argument 1 of 'stm_source_write'
     stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
                      ^~~
   In file included from drivers/hwtracing/stm/ftrace.c:18:0:
   include/linux/stm.h:136:13: note: expected 'struct stm_source_data *' but argument is of type 'struct stm_source_data'
    int notrace stm_source_write(struct stm_source_data *data, unsigned int chan,
                ^~~~~~~~~~~~~~~~

vim +/stm_source_write +49 drivers/hwtracing/stm/ftrace.c

    43	 */
    44	static void notrace
    45	stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
    46	{
    47		struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
    48	
  > 49		stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
    50	}
    51	
    52	static int stm_ftrace_link(struct stm_source_data *data)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58988 bytes --]

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]


Hi,

kbuild test robot <lkp@intel.com> writes:
> Hi Felipe,
>
> [auto build test ERROR on tip/perf/core]
> [also build test ERROR on v4.12-rc1 next-20170518]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Felipe-Balbi/kernel-trace-pass-export-pointer-as-argument-to-write/20170518-135452
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
>
> All errors (new ones prefixed by >>):
>
>    drivers/hwtracing/stm/ftrace.c: In function 'stm_ftrace_write':
>>> drivers/hwtracing/stm/ftrace.c:49:19: error: incompatible type for argument 1 of 'stm_source_write'
>      stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);
>                       ^~~
>    In file included from drivers/hwtracing/stm/ftrace.c:18:0:
>    include/linux/stm.h:136:13: note: expected 'struct stm_source_data *' but argument is of type 'struct stm_source_data'
>     int notrace stm_source_write(struct stm_source_data *data, unsigned int chan,
>                 ^~~~~~~~~~~~~~~~
>
> vim +/stm_source_write +49 drivers/hwtracing/stm/ftrace.c
>
>     43	 */
>     44	static void notrace
>     45	stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
>     46	{
>     47		struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
>     48	
>   > 49		stm_source_write(stm->data, STM_FTRACE_CHAN, buf, len);

sorry, just came back from vacations and noticed this. I'll send a v2 shortly.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH v2] kernel: trace: pass export pointer as argument to ->write()

[Felipe Balbi]

That way, users don't need to keep a global static pointer and can
rely on container_of() to fetch their own structure.

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
---

Changes since v1:
	- add missing & operator

 drivers/hwtracing/stm/ftrace.c | 6 ++++--
 include/linux/trace.h          | 2 +-
 kernel/trace/trace.c           | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/hwtracing/stm/ftrace.c b/drivers/hwtracing/stm/ftrace.c
index bd126a7c6da2..7da75644c750 100644
--- a/drivers/hwtracing/stm/ftrace.c
+++ b/drivers/hwtracing/stm/ftrace.c
@@ -42,9 +42,11 @@ static struct stm_ftrace {
  * @len:	length of the data packet
  */
 static void notrace
-stm_ftrace_write(const void *buf, unsigned int len)
+stm_ftrace_write(struct trace_export *export, const void *buf, unsigned int len)
 {
-	stm_source_write(&stm_ftrace.data, STM_FTRACE_CHAN, buf, len);
+	struct stm_ftrace *stm = container_of(export, struct stm_ftrace, ftrace);
+
+	stm_source_write(&stm->data, STM_FTRACE_CHAN, buf, len);
 }
 
 static int stm_ftrace_link(struct stm_source_data *data)
diff --git a/include/linux/trace.h b/include/linux/trace.h
index 9330a58e2651..477aed6f0caa 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -17,7 +17,7 @@
  */
 struct trace_export {
 	struct trace_export __rcu	*next;
-	void (*write)(const void *, unsigned int);
+	void (*write)(struct trace_export *, const void *, unsigned int);
 };
 
 int register_ftrace_export(struct trace_export *export);
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 1122f151466f..d0b5e8baa858 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2317,7 +2317,7 @@ trace_process_export(struct trace_export *export,
 
 	entry = ring_buffer_event_data(event);
 	size = ring_buffer_event_length(event);
-	export->write(entry, size);
+	export->write(export, entry, size);
 }
 
 static DEFINE_MUTEX(ftrace_export_lock);
-- 
2.11.0.295.gd7dffce1ce

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Steven Rostedt]

On Wed, 17 May 2017 22:52:28 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 18 May 2017 10:26:59 +0800
> Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
> 
> > On 17 May 2017 at 16:05, Felipe Balbi <felipe.balbi@linux.intel.com> wrote:  
> > > That way, users don't need to keep a global static pointer and can
> > > rely on container_of() to fetch their own structure.
> > >
> > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>    
> > 
> > Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>  
> 
> 
> If someone wants to pull this through their tree, feel free to do
> so. I've already acked it.

I guess nobody pulled this in. Does it still need to go? I'll add it to
my tree and start testing it, but I'll drop it if it's not needed (or I
don't get a response to this email).

-- Steve

Re: [PATCH] kernel: trace: pass export pointer as argument to ->write()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1205 bytes --]


Hi,

Alexander Shishkin <alexander.shishkin@linux.intel.com> writes:
> On Mon, Dec 04, 2017 at 07:09:38AM -0500, Steven Rostedt wrote:
>> On Wed, 17 May 2017 22:52:28 -0400
>> Steven Rostedt <rostedt@goodmis.org> wrote:
>> 
>> > On Thu, 18 May 2017 10:26:59 +0800
>> > Chunyan Zhang <zhang.chunyan@linaro.org> wrote:
>> > 
>> > > On 17 May 2017 at 16:05, Felipe Balbi <felipe.balbi@linux.intel.com> wrote:  
>> > > > That way, users don't need to keep a global static pointer and can
>> > > > rely on container_of() to fetch their own structure.
>> > > >
>> > > > Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>    
>> > > 
>> > > Reviewed-by: Chunyan Zhang <zhang.chunyan@linaro.org>  
>> > 
>> > 
>> > If someone wants to pull this through their tree, feel free to do
>> > so. I've already acked it.
>> 
>> I guess nobody pulled this in. Does it still need to go? I'll add it to
>> my tree and start testing it, but I'll drop it if it's not needed (or I
>> don't get a response to this email).
>
> Ouch, it somehow got lost in my inbox. I don't mind taking it through my
> tree if there are no objections.

let me know if you guys need a rebase.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]