Re: [PATCH 0/3] ramzswap: Eliminate stale data from compressed memory (v2)

[Pekka Enberg <penberg@cs.helsinki.fi>]

Hi Andrew,

Andrew Morton wrote:
> Looking at the changelogs I'm seeing no information about the
> effectiveness of ramzswap - how much memory it saves.  As that's the
> entire point of the driver, that would be a rather important thing to
> have included in the commit comments.  We cannot make the decision to
> merge ramzswap without this info.

There's some benchmarks at ramzswap pages:

http://code.google.com/p/compcache/wiki/Performance

http://code.google.com/p/compcache/wiki/SwapDiskVsRamz

[ snip bunch of comments from Andrew that need to be addressed,
   hopefully we'll get some help from the staging people ]

> The driver appears to be controlled by some nasty-looking ioctl against
> some fd.  None of it is documented anywhere.  It should be.  You're
> proposing here a permanent extension to the kernel ABI which we will
> need to maintain for ever.  That's a big deal and it is the very first
> thing reviewers will look at, before even considering the code.

I thought we got rid of it? Nitin?

> RZSIO_GET_STATS looks to be hopeless from a long-term maintainability
> POV.  It's debug code and it would be better to move it into a debugfs
> file, where we can then add and remove things at will.

Yup.

> I've completely forgotten why we need this xvmalloc thing and I don't
> recall whether we decided it would be a good thing to have as a generic
> facility and of course it's all unexplained and undocumented.  I won't
> be looking at it today, for this reason.

We need it because the slab allocator is not a good fit for this special 
purpose driver due to fragmentation. Nitin, you had a nice web page 
showing all the relevant numbers but I can't find it anymore.

Andrew, FWIW, I'm ok with xvmalloc() for this particular driver. There 
was some discussion on making it more generic but I don't see it as a 
merge-stopper for the driver.

> The overall idea and utility appear to be good and desirable, IMO.  But
> the code isn't productively reviewable in this state.

I agree that the whole graduation step from staging to kernel proper is 
not well-defined. Any suggestions? That said, I hope that doesn't stop 
us from merging this patch series because the lack of notifiers cripples 
the current ramzswap performance.

			Pekka