On Sun, Sep 01 2013 at 7:10am -0400,
Akira Hayakawa <ruby.wktk@xxxxxxxxx> wrote:
> This patch introduces dm-writeboost to staging tree.
>
> dm-writeboost is a log-structured caching software.
> It batches in-coming random-writes to a big sequential write
> to a cache device.
>
> Unlike other block caching softwares like dm-cache and bcache,
> dm-writeboost focuses on bursty writes.
> Since the implementation is optimized on writes,
> the benchmark using fio indicates that
> it performs 259MB/s random writes with
> SSD with 266MB/s sequential write throughput
> which is only 3% loss.
>
> Furthermore,
> because it uses SSD cache device sequentially,
> the lifetime of the device is maximized.
>
> The merit of putting this software in staging tree is
> to make it more possible to get feedback from users
> and thus polish the code.
>
> Signed-off-by: Akira Hayakawa <ruby.wktk@xxxxxxxxx>
> ---
> MAINTAINERS | 7 +
> drivers/staging/Kconfig | 2 +
> drivers/staging/Makefile | 1 +
> drivers/staging/dm-writeboost/Kconfig | 8 +
> drivers/staging/dm-writeboost/Makefile | 1 +
> drivers/staging/dm-writeboost/TODO | 11 +
> drivers/staging/dm-writeboost/dm-writeboost.c | 3445 +++++++++++++++++++++++
> drivers/staging/dm-writeboost/dm-writeboost.txt | 133 +
> 8 files changed, 3608 insertions(+)
> create mode 100644 drivers/staging/dm-writeboost/Kconfig
> create mode 100644 drivers/staging/dm-writeboost/Makefile
> create mode 100644 drivers/staging/dm-writeboost/TODO
> create mode 100644 drivers/staging/dm-writeboost/dm-writeboost.c
> create mode 100644 drivers/staging/dm-writeboost/dm-writeboost.txt
Hi Akira,
Sorry for not getting back to you sooner. I'll make an effort to be
more responsive from now on.
Here is a list of things I noticed during the first _partial_ pass in
reviewing the code:
- the various typedefs aren't needed (e.g. device_id, cache_id,
cache_nr)
- variable names need to be a bit more verbose (arr => array)
- really not convinced we need WB{ERR,WARN,INFO} -- may have been useful
for early development but production code shouldn't be emitting
messages with line numbers
- all the code in one file is too cumbersome; would like to see it split
into multiple files.. not clear on what that split would look like yet
- any chance the log-structured IO could be abstracted to a new class in
drivers/md/persistent-data/ ? At least factor out a library with the
interface that drives the IO.
- really dislike the use of an intermediate "writeboost-mgr" target to
administer the writeboost devices. There is no need for this. Just
have a normal DM target whose .ctr takes care of validation and
determines whether a device needs formatting, etc. Otherwise I cannot
see how you can properly stack DM devices on writeboost devices
(suspend+resume become tediously different)
- shouldn't need to worry about managing your own sysfs hierarchy;
when a dm-writeboost .ctr takes a reference on a backing or cache
device it will establish a proper hierarchy (see: dm_get_device). What
advantages are you seeing from having/managing this sysfs tree?
- I haven't had time to review the migration daemon post you made today;
but it concerns me that dm-writeboost ever required this extra service
for normal function. I'll take a closer look at what you're asking
and respond tomorrow.
But in short this code really isn't even suitable for inclusion via
staging. There are far too many things, on a fundamental interface
level, that we need to sort out.
Probably best for you to publish the dm-writeboost code a git repo on
github.com or the like. I just don't see what benefit there is to
putting code like this in staging. Users already need considerable
userspace tools and infrastructure will also be changing in the
near-term (e.g. the migration daemon).
Regards,
Mike
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
