Re: [PATCH v1 3/3] kvm tools: Add QCOW level2 caching support

[Ingo Molnar <mingo@elte.hu>]

* Pekka Enberg <penberg@kernel.org> wrote:

> On Wed, May 18, 2011 at 1:17 PM, Prasad Joshi <prasadjoshi124@gmail.com> wrote:
> > QCOW uses two tables level1 (L1) table and level2
> > (L2) table. The L1 table points to offset of L2
> > table. When a QCOW image is probed, the L1 table is
> > cached in the memory to avoid reading it from disk
> > on every reference. This caching imporves the
> > performance. The similar performance improvment can
> > be observed when L2 tables are also cached. It is
> > impossible to cache all of the L2 tables because of
> > the memory constraint. The patch adds L2 table
> > caching capability for upto 128 L2 tables, it uses
> > combination of RB tree and List to manage the L2
> > cached tables. The link list implementation helps
> > in building simple LRU structure and RB tree helps
> > in improving the search time during read/write
> > operations.
> 
> Can you please split that text into more readable paragraphs? The line
> wrapping column seems rather aggressive that's also contributing to
> unreadability.
> 
> > @@ -16,6 +16,153 @@
> >  #include <linux/kernel.h>
> >  #include <linux/types.h>
> >
> > +static inline int insert(struct rb_root *root, struct qcow_l2_cache *new)
> > +{
> > +       struct rb_node **link = &(root->rb_node), *parent = NULL;
> > +       u64 offset = new->offset;
> > +
> > +       /* search the tree */
> > +       while (*link) {
> > +               struct qcow_l2_cache *t;
> > +
> > +               t = rb_entry(*link, struct qcow_l2_cache, node);
> > +               if (!t)
> > +                       goto error;
> > +
> 
> [snip, snip]
> 
> This function and others are way to big to be marked as "inline".

yep - just leave it static and the compiler should be able to sort it out.

If not we'll write a compiler too ;-)

Thanks,

	Ingo