ureadahead patch

[Chase Douglas]

On Thu, 2010-07-08 at 19:19 -0600, Tim Gardner wrote:
> On 07/08/2010 01:46 PM, Chase Douglas wrote:
> > On Thu, 2010-07-08 at 10:24 -0600, Tim Gardner wrote:
> >> diff -u ureadahead-0.100.0/debian/changelog ureadahead-0.100.0/debian/changelog
> >> --- ureadahead-0.100.0/debian/changelog
> >> +++ ureadahead-0.100.0/debian/changelog
> >> @@ -1,3 +1,9 @@
> >> +ureadahead (0.100.0-5ubuntu1) maverick; urgency=low
> >> +
> >> +  *
> >> +
> >> + -- Tim Gardner<rtg at lochsa>   Thu, 08 Jul 2010 16:04:39 +0000
> >> +
> >>   ureadahead (0.100.0-5) maverick; urgency=low
> >>
> >>     * src/pack.c: Amend mount point detection logic to stat the mount point
> >> only in patch2:
> >> unchanged:
> >> --- ureadahead-0.100.0.orig/src/trace.c
> >> +++ ureadahead-0.100.0/src/trace.c
> >> @@ -122,6 +122,7 @@
> >>   	int                 old_open_exec_enabled = 0;
> >>   	int                 old_uselib_enabled = 0;
> >>   	int                 old_tracing_enabled = 0;
> >> +	int                 old_buffer_size_kb = 0;
> >>   	struct sigaction    act;
> >>   	struct sigaction    old_sigterm;
> >>   	struct sigaction    old_sigint;
> >> @@ -165,7 +166,7 @@
> >>
> >>   		old_uselib_enabled = -1;
> >>   	}
> >> -	if (set_value (dfd, "buffer_size_kb", 128000, NULL)<  0)
> >> +	if (set_value (dfd, "buffer_size_kb", 128000,&old_buffer_size_kb)<  0)
> >>   		goto error;
> >>   	if (set_value (dfd, "tracing_enabled",
> >>   		       TRUE,&old_tracing_enabled)<  0)
> >> @@ -217,6 +218,9 @@
> >>   	if (set_value (dfd, "events/fs/do_sys_open/enable",
> >>   		       old_sys_open_enabled, NULL)<  0)
> >>   		goto error;
> >> +	if (set_value (dfd, "buffer_size_kb",
> >> +		       old_buffer_size_kb, NULL)<  0)
> >> +		goto error;
> >>
> >>   	/* Be nicer */
> >>   	if (nice (15))
> >>
> >
> > I'm guessing that set_value copies what would be the result of "cat
> > buffer_size_kb" into old_buffer_size_kb. Unfortunately, that would end
> > up with:
> >
> > 7 (expanded: 1408)
> >
> > Or something like that. This output signifies that at first we only have
> > 7 KB/cpu of buffer, but as soon as a function trace is started we
> > allocate 1408 KB/cpu. When you manually change the value, you
> > automatically get pushed to the expanded state. This is a one-way
> > operation; we can't go back to the original state once we bump the value
> > in ureadahead.
> >
> > I think that the best thing is to just set it back to 1 (a minimum
> > value). If someone wants to do a trace, they can bump this value up. I'm
> > wary of even going back to the expanded value because in the vast
> > majority of cases we're just wasting memory.
> >
> > -- Chase
> >
> >
> 
> The first thing set_value() does is to perform atoi() on the buffer read 
> from /sys/kernel/debug/tracing/buffer_size_kb, the first token of which 
> appears to always be an integer. Isn't that the number we'd like to 
> preserve?

I think the default in that case is 7 KB/cpu, which is reasonable. Since
the set_value() implementation can handle what's read from the file, I
think this is reasonable.

-- Chase