Messages in this thread | | | Date | Sat, 08 May 2010 09:29:01 +0300 | From | Pekka Enberg <> | Subject | Re: [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
| |