Re: [patch v3]DM: dm-insitu-comp: a compressed DM target for SSD

[Mike Snitzer]

On Fri, Mar 14 2014 at  5:40am -0400,
Shaohua Li <shli@xxxxxxxxxx> wrote:

> On Mon, Mar 10, 2014 at 09:52:56AM -0400, Mike Snitzer wrote:
> > On Fri, Mar 07 2014 at  2:57am -0500,
> > Shaohua Li <shli@xxxxxxxxxx> wrote:
> > 
> > > ping!
> > 
> > Hi,
> > 
> > I intend to get dm-insitu-comp reviewed for 3.15.  Sorry I haven't
> > gotten back with you before now, been busy tending to 3.14-rc issues.
> > 
> > I took a quick first pass over your code a couple weeks ago.  Looks to
> > be in great shape relative to coding conventions and the more DM
> > specific conventions.  Clearly demonstrates you have a good command of
> > DM concepts and quirks.

Think I need to eat my words from above at least partially.  Given you
haven't implemented any of the target suspend or resume hooks this
target will _not_ work properly across suspend + resume cycles that all
DM targets must support.

But we can obviously work through it with urgency for 3.15.

I've pulled your v3 patch into git and have overlayed edits from my
first pass.  Lots of funky wrapping to conform to 80 columns.  But
whitespace aside, I've added FIXME:s in the relevant files.  If you work
on any of these FIXMEs please send follow-up patches so that we don't
step on each others' toes.

Please see the 'for-3.15-insitu-comp' branch of this git repo:
git://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git

https://git.kernel.org/cgit/linux/kernel/git/snitzer/linux.git/log/?h=for-3.15-insitu-comp

> > But one thing that would really help get dm-insitu-comp into 3.15 is to
> > show that the code is working as you'd expect.  To that end, it'd be
> > great if you'd be willing to add dm-insitu-comp support to the
> > device-mapper-test-suite, see:
> > https://github.com/jthornber/device-mapper-test-suite
> > 
> > I recently added barebones/simple dm-crypt support, see:
> > https://github.com/jthornber/device-mapper-test-suite/commit/c865bcd4e48228e18626d94327fb2485cf9ec9a1
> > 
> > But It may be that activation/test code for the other targets (e.g. thin
> > or cache) are more useful examples to follow for implemnting
> > dm-insitu-comp stack activation, see:
> > https://github.com/jthornber/device-mapper-test-suite/blob/master/lib/dmtest/pool-stack.rb
> > https://github.com/jthornber/device-mapper-test-suite/blob/master/lib/dmtest/cache_stack.rb
> > 
> > All said, implementing dm-insitu-comp support for dmts (including some
> > tests that establish it is working as intended) isn't a hard requirement
> > for getting the target upstream but it would _really_ help.
> 
> Ok, I added some simple tests in the test suites.

OK, I missed this before because it was an attachment.  I was confused
as to whether you already added or will add support.  Now that I've
replied to this mail mutt pulled in the attachment ;)

I'll take for a spin on Monday (or over the weekend if I'm bored).

Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/