Re: [PATCH] perf, tool, record: Fix the header generation for pipe

From: Jiri Olsa
Date: Mon Aug 29 2011 - 09:42:10 EST


On Mon, Aug 29, 2011 at 10:20:37AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Aug 22, 2011 at 06:07:13PM +0200, Jiri Olsa escreveu:
> > On Mon, Aug 22, 2011 at 05:51:06PM +0200, Eric Dumazet wrote:
> > > Le lundi 22 août 2011 à 16:52 +0200, Jiri Olsa a écrit :
> > > > - fd is always file.. the descriptor, which might be pipe, is in output_fd variable
> > > > but, maybe the die call is not necessary.. this should not fail
>
> > > > - the record_file function is called only on debugfs or procfs files:
> > > > events/header_page
> > > > events/header_event
> > > > events/**/format
> > > > printk_formats
> > > > /proc/kallsyms
>
> > > > so I think I need to read the whole file as in current code.
>
> > > You read the file twice. Is it the right fix ?
>
> > > Once to compute the length to be able to write the output header, once
> > > to process the content.
>
> > > Are you sure length cannot change between the two phases ?
>
> > > /proc/kallsyms can definitely change when a module is loaded.
>
> > Right, better to store it to a temp file as you suggest, so we have
> > consistent view.. I'll prepare new patch.
>
> Hi Jiri, any news here?
>
> I just stumbled in this bug while testing Neil's net_dropmonitor script
> :-\

well, I was thinking about taking files snapshot, so we dont read them
twice and omit the reading for size.. but it'll be probably bigger
change, I'll try to send out something by the end of the week

if you need workaround, the initial patch should work

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