All of lore.kernel.org
 help / color / mirror / Atom feed
From: Akira Hayakawa <ruby.wktk@gmail.com>
To: agk@redhat.com
Cc: dm-devel@redhat.com, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org, kernelnewbies@kernelnewbies.org
Subject: Re: [dm-devel] [RFC] dm-writeboost: plan to go to staging
Date: Sat, 31 Aug 2013 23:32:55 +0900	[thread overview]
Message-ID: <5221FE97.1050504@gmail.com> (raw)
In-Reply-To: <20130829033023.GH4872@agk-dp.fab.redhat.com>

Thanks, Alasdair.

I will reply to some of your comments
that is not answered up to now.

> The documentation file will eventually need rewriting to follow the same
> format as the other targets recently added to the kernel.  We document
> the kernel interface rather than any particular userspace tools, which
> just have the status of convenient examples.
dm-writeboost can be used by kicking kernel interfaces but
it is not recommended.
My userspace tools take care of all the
implications in using the kernel interfaces.
The fact dm-writeboost supports shared-caching and
write-back caching with auto-modulated migration
puts the interfaces and implementations under some constraints.
That's simply a tradeoff.

OK, I will document the kernel interfaces
and also mention the userspace tools.

> (Your code also needs many more comments inline to explain what it does
> and how it works.)
Comments are substantially noise is my philosophy.
I will add comments with given feedback clearly mentioning
what is clear and what is not clear about the source code.
Even in that case,
I'd better polish the code rather than write comments.

> Another little thing I noticed: look into using something like
> __dm_bless_for_disk() for your metadata and clearly segregate your
> on-disk structures and document the layout.
I couldn't understand what is __dm_bless_for_disk() for.
What does the word "bless" mean in this context?

By the way,
dm-cache and dm-writeboost are definitely different in a way that
dm-cache has an another disk for metadata while dm-writeboost doesn't.
dm-writeboost packs metadata and data within a log and
submit it to the cache device like log-structured filesystem.

So, the annotation checks can not be appropriated
to apply to dm-writeboost.

And what about moving these macros to
include/linux/device-mapper.h?

Akira

On 8/29/13 12:30 PM, Alasdair G Kergon wrote:
> On Wed, Aug 28, 2013 at 07:05:55PM -0700, Greg KH wrote:
>> For staging drivers, I need a TODO file that lists
>> what needs to be done to the code to get it into a mergable state for
>> the "real" part of the kernel,
> 
> Two simple requirements before putting your proof-of-concept into staging
> if you want to work that way:
> 
> 1) Drop the major version number to 0.  Version 1 is reserved for
> supported modules.
> 
> 2) Agree a new and meaningful target name with us so you don't have to
> change it later.  "lc" means nothing, I'm afraid.
> 
> Then in general terms, you should continue to compare your device-mapper
> target with the existing targets and where there are differences, either
> change your target to be like something that already exists, or be ready
> to explain why that can't or shouldn't be done.
> 
> In particular, the interface and architecture will need substantial
> changes and working these out should be your highest priority.
> 
> For example, you write:
> 
>   Be careful, you MUST create all the LVs as the destinations of
>   the dirty blocks on the cache device before this operation.  Otherwise,
>   the kernel may crash.
> 
> I read a statement like that as an indication of an interface or
> architectural problem.  The device-mapper approach is to 'design out'
> problems, rather than relying on users not doing bad things.
> Study the existing interfaces used by other targets to understand
> some approaches that proved successful, then decide which ones
> come closest to your needs.
> 
> (Your code also needs many more comments inline to explain what it does
> and how it works.)
> 
> The documentation file will eventually need rewriting to follow the same
> format as the other targets recently added to the kernel.  We document
> the kernel interface rather than any particular userspace tools, which
> just have the status of convenient examples.
> 
> Another little thing I noticed: look into using something like
> __dm_bless_for_disk() for your metadata and clearly segregate your
> on-disk structures and document the layout.
> 
> Alasdair
> 


      parent reply	other threads:[~2013-08-31 14:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-29  1:35 [RFC] dm-lc: plan to go to staging Akira Hayakawa
2013-08-29  2:05 ` Greg KH
2013-08-29  3:30   ` [dm-devel] " Alasdair G Kergon
2013-08-29 14:45     ` Mikulas Patocka
2013-08-30 12:33       ` Akira Hayakawa
2013-08-30 12:50         ` Alasdair G Kergon
2013-08-30 14:41           ` Akira Hayakawa
2013-08-30 14:47         ` Valdis.Kletnieks
2013-08-30 14:47           ` Valdis.Kletnieks at vt.edu
2013-08-31 14:25           ` [dm-devel] [RFC] dm-writeboost: " Akira Hayakawa
2013-08-31 14:32     ` Akira Hayakawa [this message]

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=5221FE97.1050504@gmail.com \
    --to=ruby.wktk@gmail.com \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=linux-kernel@vger.kernel.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.