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    [W:0.022 / U:119.540 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site