Re: [PATCH] Fix writing to trace/trace_options

[Ingo Molnar <mingo@elte.hu>]

* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:

> On Mon, Nov 17, 2008 at 12:17:47PM +0100, Ingo Molnar wrote:
> > 
> > * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > 
> > > On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> > > > writing to trace/trace_options use the index of the array
> > > > to find the value of the flag. With branch tracer flag
> > > > defined conditionally, this breaks writing to trace_options
> > > > with branch tracer disabled.
> > > > 
> > > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > > 
> > > tg export didn't add numbering to the patch based on dependency.
> > > So the series is in the below order
> > > 
> > > [PATCH] ftrace: add proper bin iterator support
> > > [PATCH] ftrace: Add dump iteator
> > > [PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
> > > [PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
> > > [PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
> > > [PATCH] Fix writing to trace/trace_options
> > > 
> > > The patches are against -tip with HEAD
> > > 7195b6707adcd00f413ce07e6b9954b4c597495c
> > 
> > hm, i'm not sure about this. We already do binary dumping, but only 
> > for the cases where we actually know the structure of the data (i.e. 
> > binary dumping is just an output format, not a tracing type). And that 
> > is good so.
> 
> Why do we need to limit to know structures. debug_dump can be looked at
> as a debugging helper which allows the user to send more data in binary
> format. Later user space can decide to look at the values. I had the
> below test case done to check the patches.
> 
>  	int err;
> +	struct data {
> +		char i;
> +		int k;
> +	};
> +	struct data mydata = {.i = 'c', .k = 10};
>  
>  	/*
>  	 * If we have encountered a bitmap-format file, the size limit
>  	 * is smaller than s_maxbytes, which is for extent-mapped files.
>  	 */
> +	dp_printk("%s with value %d\n", __func__, pos);
> +	debug_dump(&mydata, sizeof(mydata));
> 
> 
> > 
> > In your patchset right now nothing uses debug_dump(ptr, len) so 
> > it's hard to see exactly how we should shape it. What specific 
> > usages do you have in mind?
> 
> If you are not convinced about debug_dump you may want to pick the 
> first three patches that include a bug fix and support for 
> dp_printk.

i'm still wondering about the specific usecase you have in mind. Where 
do we want to dump unstructured data to user-space? If you look at all 
the existing ftrace plugins we know the structure of the logged data 
very well, and most of the plugins have semantic dependencies on that 
data content as well.

and having binary log data that _only_ user-space can process 
meaningfully, while it's opaque to the tracer plugin, sounds like a 
poor concept to me.

	Ingo