All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: Alberto Garcia <berto@igalia.com>,
	Ed Swierk <eswierk@skyportsystems.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH] qcow2: Optimize L2 table cache size based on image and cluster sizes
Date: Wed, 5 Oct 2016 17:24:08 +0200	[thread overview]
Message-ID: <20161005152408.GH1657@noname.str.redhat.com> (raw)
In-Reply-To: <2e3185e2-6a56-5c1a-eff1-5331d817fc3c@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2425 bytes --]

Am 05.10.2016 um 17:13 hat Max Reitz geschrieben:
> On 05.10.2016 16:57, Alberto Garcia wrote:
> > On Tue 04 Oct 2016 05:51:26 PM CEST, Max Reitz wrote:
> > 
> >>> At least giving users a way to skip the math would be an improvement.
> >>> Would you be okay with an explicitly-set option like
> >>> l2_cache_size=auto or =max that optimizes for performance at the
> >>> expense of memory?
> >>
> >> That (cache-size=max) is actually something Stefan Hajnoczi has
> >> proposed at KVM Forum 2015.
> >>
> >> I agree that implementing the formula in qemu's qcow2 driver itself
> >> makes sense and will help users; however, I do think it is appropriate
> >> to expect the user to pass cache-size=max if they need it.
> > 
> > Frank Myhr's suggestion (in bugzilla) is that we allow specifying a % of
> > the disk size, so
> > 
> > l2-cache-size=100%  (that would be cache-size=max)
> > l2-cache-size=80%
> > ...
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1377735#c3
> > 
> > Either one looks good to me. They would change however the data type of
> > 'l2-cache-size' from int to string in BlockdevOptionsQcow2, can we
> > actually do that?
> 
> I think we can do that with an alternate data type which accepts both an
> integer and a string. If we only had "max", we'd probably want to make
> it an enumeration instead of a free-form string, though. But with a %
> suffix, we'd probably need a string.
> 
> For me, either works fine.

I'm not sure if we want to support such syntax in QMP. It sounds like a
typical convenience option for human users.

> Apart from that, I have to say I think it would be a bit more useful if
> one would specify the area covered by the metadata caches as an absolute
> number instead of a relative one (I guess it's generally easier to know
> what area your applications will perform random accesses on than the
> relative size, but maybe that's just me).
> 
> Supporting that with cache-size is difficult, though, because how are
> you going to decide between whether the user is specifying the actual
> size of the cache or the area it covers?
> 
> So maybe it would make more sense to introduce a whole new set of
> options called {,l2-,refcount-}cache-coverage or something. These
> options would then accept both an absolute and a relative number.

If we want easy, make it easy: cache-coverage and apply it to both.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

  parent reply	other threads:[~2016-10-05 15:24 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-04 13:39 [Qemu-devel] [PATCH] qcow2: Optimize L2 table cache size based on image and cluster sizes Ed Swierk
2016-10-04 14:31 ` [Qemu-devel] [Qemu-block] " Alberto Garcia
2016-10-04 15:36   ` Ed Swierk
2016-10-04 15:51     ` Max Reitz
2016-10-05 14:57       ` Alberto Garcia
2016-10-05 15:13         ` Max Reitz
2016-10-05 15:19           ` Alberto Garcia
2016-10-05 15:35             ` Max Reitz
2016-10-06 14:34               ` Alberto Garcia
2016-10-05 15:24           ` Kevin Wolf [this message]
2016-10-04 18:19 ` [Qemu-devel] [PATCH v2] qcow2: Add cache-size=max option to optimize caches for performance Ed Swierk
2016-10-04 21:50   ` no-reply
     [not found] <70fbc326-29b6-ca3c-54ed-201b46d71cf0@larkmoor.net>
2016-10-07  8:14 ` [Qemu-devel] [Qemu-block] [PATCH] qcow2: Optimize L2 table cache size based on image and cluster sizes Kevin Wolf
2016-10-07 10:56   ` Alberto Garcia
2016-10-07 13:46     ` Frank Myhr
2016-10-07 13:58       ` Ed Swierk
2016-10-18 15:25         ` Alberto Garcia
2016-10-19 16:34           ` Frank Myhr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161005152408.GH1657@noname.str.redhat.com \
    --to=kwolf@redhat.com \
    --cc=berto@igalia.com \
    --cc=eswierk@skyportsystems.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.