Re: [PATCH] Fix writing to trace/trace_options

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

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.

-aneesh