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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
