linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Thornber <thornber@redhat.com>
To: Akira Hayakawa <ruby.wktk@gmail.com>
Cc: mpatocka@redhat.com, dm-devel@redhat.com,
	devel@driverdev.osuosl.org, snitzer@redhat.com,
	gregkh@linuxfoundation.org, david@fromorbit.com,
	linux-kernel@vger.kernel.org, dan.carpenter@oracle.com,
	joe@perches.com, akpm@linux-foundation.org, m.chehab@samsung.com,
	ejt@redhat.com, agk@redhat.com, cesarb@cesarb.net
Subject: Re: [dm-devel] dm-writeboost testing
Date: Fri, 4 Oct 2013 16:03:14 +0100	[thread overview]
Message-ID: <20131004150313.GB25523@debian> (raw)
In-Reply-To: <524D70DA.8040308@gmail.com>

On Thu, Oct 03, 2013 at 10:27:54PM +0900, Akira Hayakawa wrote:

> > dm-cache doesn't have this problem, if you overwrite the same piece of 
> > data again and again, it goes to the cache device.
> 
> It is not a bug but should/can be optimized.
> 
> Below is the cache hit path for writes.
> writeboost performs very poorly when a partial write hits
> which then turns `needs_cleanup_perv_cache` to true.

Are you using fixed size blocks for caching then?  The whole point of
using a journal/log based disk layout for caching is you can slurp up
all writes irrespective of their size.

What are the scenarios where you out perform dm-cache?

- Joe




> Partial write hits is believed to be unlikely so
> I decided to give up this path to make other likely-paths optimized.
> I think this is just a tradeoff issue of what to be optimized the most.
> 
>         if (found) {
> 
>                 if (unlikely(on_buffer)) {
>                         mutex_unlock(&cache->io_lock);
> 
>                         update_mb_idx = mb->idx;
>                         goto write_on_buffer;
>                 } else {
>                         u8 dirty_bits = atomic_read_mb_dirtiness(seg, mb);
> 
>                         /*
>                          * First clean up the previous cache
>                          * and migrate the cache if needed.
>                          */
>                         bool needs_cleanup_prev_cache =
>                                 !bio_fullsize || !(dirty_bits == 255);
> 
>                         if (unlikely(needs_cleanup_prev_cache)) {
>                                 wait_for_completion(&seg->flush_done);
>                                 migrate_mb(cache, seg, mb, dirty_bits, true);
>                         }
> 
> I checked that the mkfs.ext4 writes only in 4KB size
> so it is not gonna turn the boolean value true for going into the slowpath.
> 
> Problem:
> Problem is that
> it chooses the slowpath even though the bio is full-sized overwrite
> in the test.
> 
> The reason is that the dirty bits is sometimes seen as 0
> and the suspect is the migration daemon.
> 
> I guess you created the writeboost device with the default configuration.
> In that case migration daemon always works and
> some metadata is cleaned up in background.
> 
> If you turns both enable_migration_modulator and allow_migrate to 0
> before beginning the test
> to stop migration at all
> it never goes into the slowpath with the test.
> 
> Solution:
> Changing the code to
> avoid going into the slowpath when the dirty bits is zero
> will solve this problem.
> 
> And done. Please pull the latest one from the repo.
> --- a/Driver/dm-writeboost-target.c
> +++ b/Driver/dm-writeboost-target.c
> @@ -688,6 +688,14 @@ static int writeboost_map(struct dm_target *ti, struct bio *bio
>                         bool needs_cleanup_prev_cache =
>                                 !bio_fullsize || !(dirty_bits == 255);
> 
> +                       /*
> +                        * Migration works in background
> +                        * and may have cleaned up the metablock.
> +                        * If the metablock is clean we need not to migrate.
> +                        */
> +                       if (!dirty_bits)
> +                               needs_cleanup_prev_cache = false;
> +
>                         if (unlikely(needs_cleanup_prev_cache)) {
>                                 wait_for_completion(&seg->flush_done);
>                                 migrate_mb(cache, seg, mb, dirty_bits, true);
> 
> Thanks,
> Akira

  reply	other threads:[~2013-10-04 15:06 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-03  0:15 dm-writeboost testing Mikulas Patocka
2013-10-03 13:27 ` [dm-devel] " Akira Hayakawa
2013-10-04 15:03   ` Joe Thornber [this message]
2013-10-04  2:28 ` Akira Hayakawa
2013-10-04 13:38   ` Mikulas Patocka
2013-10-04 14:25     ` Akira Hayakawa
2013-10-04 15:56       ` Mikulas Patocka
2013-10-05  7:05         ` Akira Hayakawa
2013-10-06  0:47           ` Mikulas Patocka
2013-10-06  2:14             ` Akira Hayakawa

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=20131004150313.GB25523@debian \
    --to=thornber@redhat.com \
    --cc=agk@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=cesarb@cesarb.net \
    --cc=dan.carpenter@oracle.com \
    --cc=david@fromorbit.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=dm-devel@redhat.com \
    --cc=ejt@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.chehab@samsung.com \
    --cc=mpatocka@redhat.com \
    --cc=ruby.wktk@gmail.com \
    --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).