From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH v1 3/3] kvm tools: Add QCOW level2 caching support
Date: Wed, 18 May 2011 13:10:45 +0200
Message-ID: <20110518111045.GD16556@elte.hu>
References: <1305713837-18889-1-git-send-email-prasadjoshi124@gmail.com>
 <1305713837-18889-3-git-send-email-prasadjoshi124@gmail.com>
 <BANLkTikYakE=82Ag=FoaX3VF0=TAUdPbRw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Prasad Joshi <prasadjoshi124@gmail.com>, kvm@vger.kernel.org,
	asias.hejun@gmail.com, gorcunov@gmail.com, levinsasha928@gmail.com,
	chaitanyakulkarni15@gmail.com, ashwini.kulkarni@gmail.com
To: Pekka Enberg <penberg@kernel.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.mail.elte.hu ([157.181.151.9]:39113 "EHLO mx2.mail.elte.hu"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S932763Ab1ERLKy (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 18 May 2011 07:10:54 -0400
Content-Disposition: inline
In-Reply-To: <BANLkTikYakE=82Ag=FoaX3VF0=TAUdPbRw@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>


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

> On Wed, May 18, 2011 at 1:17 PM, Prasad Joshi <prasadjoshi124@gmail.c=
om> 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.
>=20
> Can you please split that text into more readable paragraphs? The lin=
e
> wrapping column seems rather aggressive that's also contributing to
> unreadability.
>=20
> > @@ -16,6 +16,153 @@
> > =A0#include <linux/kernel.h>
> > =A0#include <linux/types.h>
> >
> > +static inline int insert(struct rb_root *root, struct qcow_l2_cach=
e *new)
> > +{
> > + =A0 =A0 =A0 struct rb_node **link =3D &(root->rb_node), *parent =3D=
 NULL;
> > + =A0 =A0 =A0 u64 offset =3D new->offset;
> > +
> > + =A0 =A0 =A0 /* search the tree */
> > + =A0 =A0 =A0 while (*link) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct qcow_l2_cache *t;
> > +
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 t =3D rb_entry(*link, struct qcow_l2_=
cache, node);
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!t)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto error;
> > +
>=20
> [snip, snip]
>=20
> 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 o=
ut.

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

Thanks,

	Ingo