lkml.org 
[lkml]   [2010]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/3] ramzswap: Eliminate stale data from compressed memory (v2)
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


\
 
 \ /
  Last update: 2010-05-08 08:33    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site