Re: RFC: seq_file: optimize memory re-alloc strategy for seq file sizes > PAGE_SIZE

[srinivas kandagatla <srinivas.kandagatla@gmail.com>]

Hi Greg,
Thanks for the comments,
I agree with you on performance vs debugging code.
However actual issue I want to stress on is iterator restarting
multiple times and not the performance.

thanks,
srini

On Fri, Jul 15, 2011 at 4:18 PM, Greg KH <gregkh@suse.de> wrote:
>
> On Fri, Jul 15, 2011 at 09:51:32AM +0100, Srinivas KANDAGATLA wrote:
> > Hi All,
> > I have noticed that, reading or dumping a full sysfs entry (ex:
> > /sys/kernel/debug/gpio )
>
> That's not sysfs, it's debugfs.
>
> > which exceeds PAGE_SIZE restarts associated seq_file iterator
> > (calling start->show->stop)
> > morethan one time.
> >
> > In my case gpio entry just exceeded 8k which resulted in show
> > function getting
> > called 3 times.
> >
> > The reason is because the allocated buffer overflows and seq_file allocates
> > new buffer of next order and starts iteration from the start, which
> > is clear in
> > seq_file code.
> > However this is bit of conservative approach which results in:
> > - restarting iterator if file sz exceeds PAGE_SIZE( which is really
> > not necessary )
> > - not reusing the formated buffer (which can be done by re-alloc and copy).
> > - Will help entires like dumping cacheline or pagetables to capture
> > its state with better accuracy.
> >
> > To address this situation, I think seq_file formating/printf
> > functions should
> > re-allocate buffer as soon as they detect buffer overflow.
>
> This is debugging code, why should performance matter?
>
> > This patch attempts to do the same by adding new function
> > buf_realloc which inturn
> > calls krealloc and all the seq_file formating or printing functions
> > call this
> > function whenever an overflow is detected and retry.
> >
> >
> > Main motive of this patch is to avoid restarting the iterator and NOT
> > performance, however there is a bit of gain in performance too.
> >
> > Performance figures(average over 100 times loop) for different sizes of
> > sysfs entries:
> >
> > without realloc:
> > ---------------------------------
> >   SZ        REAL    USER    SYS
> > ---------------------------------
> > | 4k   | 0.012    0.002    0.009
> > | 8k   | 0.013    0.002    0.010
> > | 16k  | 0.015    0.003    0.012
> > | 32k  | 0.019    0.002    0.017
> > --------------------------------
> >
> > with realloc:
> > ---------------------------------
> >   SZ        REAL    USER    SYS
> > ---------------------------------
> > | 4k  |    0.012    0.002    0.009
> > | 8k  |    0.012    0.002    0.009
> > | 16k |    0.013    0.002    0.010
> > | 32k |    0.015    0.003    0.012
> > -------------------------------
> >
> >
> > I discovered this issue while I was working on a different issue as
> > I happed
> > to stick a printk in show function, which got called 3 times when I
> > dumped the
> > entry with 'cat' :-). which I did not expect.
> >
> >
> > comments? :-)
>
> Again, debugfs doesn't matter for this type of thing, so adding
> complexity to the core kernel code for it shouldn't be done unless
> absolutely needed.
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html