linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Thornber <thornber@redhat.com>
To: device-mapper development <dm-devel@redhat.com>,
	gregkh@linuxfoundation.org, snitzer@redhat.com, agk@redhat.com,
	linux-kernel@vger.kernel.org
Subject: Re: [dm-devel] [PATCH] staging: writeboost: Add dm-writeboost
Date: Wed, 10 Dec 2014 10:00:33 +0000	[thread overview]
Message-ID: <20141210100033.GA21108@debian> (raw)
In-Reply-To: <20141209151253.GA17660@debian>

On Tue, Dec 09, 2014 at 03:12:53PM +0000, Joe Thornber wrote:
> Writeboost is significantly slower than the spindle alone for this
> very simple test.  I do not understand what is causing the issue.

I started doing the code review and now understand what's going on,
sadly.

You are splitting all bios up into 4k blocks to simplify the metadata
layout, and mapping logic.  This murders performance.  File systems
and the block layer try really hard to submit the largest bio possible
for a reason.

A simple dd in large chunks across your cache reveals this:

raw spindle:        8.9s
writeboost type 0:  32.2s
writeboost type 1:  71.1s

dm-cache and dm-thin do also split io into blocks, but much larger,
user configurable blocks.  It's still a performance issue for us,
which is why I'm using range locking to move away from this bio
splitting (eg, recent cache discard patches).

One of the main advantages of a log based metadata layout is you can
cope nicely with arbitrarily sized bios.  Unlike dm-cache for
instance, which has to do a read from the origin if it wants to cache
a write that partially covers a block (or maintain a 'valid' bit for
each sector of every cached block).

The writeboost target as it stands will only benefit v. small, random
io.  It will seriously degrade performance of any other IO profile.
I'm NACKing this for upstream, and will not be spending any more time
on it at this point.

You've put a lot of effort into this so far, so I suggest you redesign
the log metadata, and drop the io splitting; you'll end up with
something far better.

Sorry,

- Joe

  parent reply	other threads:[~2014-12-10 10:00 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-07 12:35 [PATCH] staging: writeboost: Add dm-writeboost Akira Hayakawa
2014-12-07 20:08 ` Greg KH
2014-12-07 21:04   ` Akira Hayakawa
2014-12-09 15:12     ` [dm-devel] " Joe Thornber
2014-12-09 15:48       ` Mike Snitzer
2014-12-10 10:21         ` Akira Hayakawa
2014-12-10 10:00       ` Joe Thornber [this message]
2014-12-10 11:00         ` [dm-devel] [PATCH] " Akira Hayakawa
2014-12-10 11:22           ` Joe Thornber
2014-12-10 12:33           ` Joe Thornber
2014-12-10 12:59             ` Akira Hayakawa
2014-12-10 13:13               ` Joe Thornber
2014-12-10 13:31                 ` Akira Hayakawa
2014-12-10 13:42                   ` Joe Thornber
2014-12-10 14:43                     ` Akira Hayakawa
2014-12-12 12:51         ` Marian Csontos

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=20141210100033.GA21108@debian \
    --to=thornber@redhat.com \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).