lkml.org 
[lkml]   [2011]   [Nov]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [GIT PULL] mm: frontswap (for 3.2 window)
    On Tue, Nov 01, 2011 at 09:41:38AM -0700, Dan Magenheimer wrote:
    > I suppose this documentation (note, it is in drivers/staging/zcache,
    > not in the proposed frontswap patchset) could be misleading. It is

    Yep I gotten the comment from tmem.c in staging, and the lwn link I
    read before reading the tmem_put comment also only mentioned about
    tmem_put doing a copy. So I erroneously assumed that all memory
    passing through tmem was being copied and you lost reference of the
    "struct page" when it entered zcache.

    But instead there is this obscure cast of a "struct page *" to a "char
    *", that is casted back to a struct page * from a char * in zcache
    code, and kmap() runs on the page, to avoid the unnecessary copy.

    So far so good, now the question is why do you have that cast at all?

    I mean it's hard to be convinced on the sanity of on a API that
    requires the caller to cast a "struct page *" to a "char *" to run
    zerocopy. And well that is the very core tmem_put API I'm talking
    about.

    I assume the explanation of the cast is: before it was passing
    page_address(page) to tmem, but that breaks with highmem because
    highmem requires kmap(page). So then you casted the page.

    This basically proofs the API must be fixed. In the kernel we work
    with _pages_ not char *, exactly for this reason, and tmem_put must be
    fixed to take a page structure. (in fact better would be an array of
    pages and ranges start/end for each entry in the array but hey at
    least a page+len would be sane). A char * is flawed and the cast of
    the page to char * and back to struct page, kind of proofs it. So I
    think that must be fixed in tmem_put. Unfortunately it's already
    merged with this cast back and forth in the upstream kernel.

    About the rest of zcache I think it's interesting but because it works
    inside tmem I'm unsure how we're going to write it to disk.

    The local_irq_save would be nice to understand why it's needed for
    frontswap but not for pagecache. All that VM code never runs from
    irqs, so it's hard to see how the irq disabling is relevant. A bit fat
    comment on why local_irq_save is needed in zcache code (in staging
    already) would be helpful. Maybe it's tmem that can run from irq? The
    only thing running from irqs is the tlb flush and I/O completion
    handlers, everything else in the VM isn't irq/softirq driven so we
    never have to clear irqs.

    My feeling is this zcache should be based on a memory pool abstraction
    that we can write to disk with a bio and working with "pages".

    I'm also not sure how you balance the pressure in the tmem pool, when
    you fail the allocation and swap to disk, or when you keep moving to
    compressed swap.

    > This is a known problem: zcache is currently not very
    > good for high-response RT environments because it currently
    > compresses a page of data with interrupts disabled, which
    > takes (IIRC) about 20000 cycles. (I suspect though, without proof,
    > that this is not the worst irq-disabled path in the kernel.)

    That's certainly more than the irq latency so it's probably something
    the rt folks don't want and yes they should keep it in mind not to use
    frontswap+zcache in embedded RT environments.

    Besides there was no benchmark comparing zram performance to zcache
    performance so latency aside we miss a lot of info.

    > As noted earlier, this is fixable at the cost of the extra copy
    > which could be implemented as an option later if needed.
    > Or, as always, the RT folks can just not enable zcache.
    > Or maybe smarter developers than me will find a solution
    > that will work even better.

    And what is the exact reason of the local_irq_save for doing it
    zerocopy?

    > Yeah, remember zcache was merged before either cleancache or
    > frontswap, so this ugliness was necessary to get around the
    > chicken-and-egg problem. Zcache will definitely need some
    > work before it is ready to move out of staging, and your
    > feedback here is useful for that, but I don't see that as
    > condemning frontswap, do you?

    Would I'd like is a mechanism where you:

    1) add swapcache to zcache (with fallback to swap immediately if zcache
    allocation fails)

    2) when some threshold is hit or zcache allocation fails, we write the
    compressed data in a compact way to swap (freeing zcache memory),
    or swapcache directly to swap if no zcache is present

    3) newly added swapcache is added to zcache (old zcache was written to
    swap device compressed and freed)

    Once we already did the compression it's silly to write to disk the
    uncompressed data. Ok initially it's ok because compacting the stuff
    on disk is super tricky but we want a design that will allow writing
    the zcache to disk and add new swapcache to zcache, instead of the
    current way of swapping the new swapcache to disk uncompressed and not
    being able to writeout the compressed zcache.

    If nobody called zcache_get and uncompressed it, it means it's
    probably less likely to be used than the newly added swapcache that
    wants to be compressed.

    I'm afraid adding frontswap in this form will still get stuck us in
    the wrong model and most of it will have to be dropped and rewritten
    to do just the above 3 points I described to do proper swap
    compression.

    Also I'm skeptical we need to pass through tmem at all to do that. I
    mean done right the swap compression could be a feature to enable
    across the board without needing tmem at all. Then if you want to add
    ramster just add a frontswap on the already compressed
    swapcache... before it goes to the hard swap device.

    The final swap design must also include the pre-swapout from Avi by
    writing data to swapcache in advance and relaying on the dirty bit to
    rewrite it. And the pre-swapin as well (original idea from Con). The
    pre-swapout would need to stop before compressing. The pre-swapin
    should stop before decompressing.

    I mean I see an huge potential for improvement in the swap space, just
    I guess most are busy with more pressing issues, like James said most
    data centers don't use swap, desktop is irrelevant and android (as
    relevant as data center) don't use swap.

    But your improvement to frontswap don't look the right direction if
    you really want to improve swap for the long term. It may be better
    than nothing but I don't see it going the way it should go and I
    prefer to remove the tmem dependency on zcache all together. Zcache
    alone would be way more interesting.

    And tmem_put must be fixed to take a page, that cast to char * of a
    page, to avoid crashing on highmem is not allowed.

    Of course I didn't have the time to read 100% of the code so please
    correct me again if I misunderstood something.

    > This is the "fix highmem" bug fix from Seth Jennings. The file
    > tmem.c in zcache is an attempt to separate out the core tmem
    > functionality and data structures so that it can (eventually)
    > be in the lib/ directory and be used by multiple backends.
    > (RAMster uses tmem.c unchanged.) The code in tmem.c reflects
    > my "highmem-blindness" in that a single pointer is assumed to
    > be able to address the "PAMPD" (as opposed to a struct page *
    > and an offset, necessary for a 32-bit highmem system). Seth
    > cleverly discovered this ugly two-line fix that (at least for now)
    > avoided major mods to tmem.c.

    Well you need to do the major mods, it's not ok to do that cast,
    passing pages is correct instead. Let's fix the tmem_put API before
    people can use it wrong. Maybe then I'll dislike passing through tmem
    less? Dunno.

    int tmem_put(struct tmem_pool *pool, struct tmem_oid *oidp, uint32_t index,
    - char *data, size_t size, bool raw, bool ephemeral)
    + struct page *page, size_t size, bool raw, bool ephemeral)


    > First ignoring frontswap, there is currently no way to move a
    > page of swap data from one swap device to another swap device
    > except by moving it first into RAM (in the swap cache), right?

    Yes.

    > Frontswap doesn't solve that problem either, though it would
    > be cool if it could. The "partial swapoff" functionality
    > in the patch, added so that it can be called from frontswap_shrink,
    > enables pages to be pulled out of frontswap into swap cache
    > so that they can be moved if desired/necessary onto a real
    > swap device.

    The whole logic deciding the size of the frontswap zcache is going to
    be messy. But to do the real swapout you should not pull the memory
    out of frontswap zcache, you should write it to disk compacted and
    compressed compared to how it was inserted in frontswap... That would
    be the ideal.

    > The selfballooning code in drivers/xen calls frontswap_shrink
    > to pull swap pages out of the Xen hypervisor when memory pressure
    > is reduced. Frontswap_shrink is not yet called from zcache.

    So I wonder how zcache is dealing with the dynamic size. Or has it a
    fixed size? How do you pull pages out of zcache to max out the real
    RAM availability?

    > Note, however, that unlike swap-disks, compressed pages in
    > frontswap CAN be silently moved to another "device". This is
    > the foundation of RAMster, which moves those compressed pages
    > to the RAM of another machine. The device _could_ be some
    > special type of real-swap-disk, I suppose.

    Yeah you can do ramster with frontswap+zcache but not writing the
    zcache to disk into the swap device. Writing to disk doesn't require
    new allocations. Migrating to other node does. And you must deal with
    OOM conditions there. Or it'll deadlock. So the basic should be to
    write compressed data to disk (which at least can be done reliably for
    swapcache, unlike ramster which has the same issues of nfs swapping
    and nbd swapping and iscsi sapping) before wondering if to send it to
    another node.

    > Yes, this is a good example of the most important feature of
    > tmem/frontswap: Every frontswap_put can be rejected for whatever reason
    > the tmem backend chooses, entirely dynamically. Not only is it true
    > that hardware can't handle this well, but the Linux block I/O subsystem
    > can't handle it either. I've suggested in the frontswap documentation
    > that this is also a key to allowing "mixed RAM + phase-change RAM"
    > systems to be useful.

    Yes what is not clear is how the size of the zcache is choosen.

    > Also I think this is also why many linux vm/vfs/fs/bio developers
    > "don't like it much" (where "it" is cleancache or frontswap).
    > They are not used to losing control of data to some other
    > non-kernel-controlled entity and not used to being told "NO"
    > when they are trying to move data somewhere. IOW, they are
    > control freaks and tmem is out of their control so it must
    > be defeated ;-)

    Either tmem works on something that is a core MM structure and is
    compatible with all bios and operations we can want to do on memory,
    or I've an hard time to think it's a good thing in trying to make the
    memory it handles not-kernel-controlled.

    This non-kernel-controlled approach to me looks like exactly a
    requirement coming from Xen, not really something useful.

    There is no reason why a kernel abstraction should stay away from
    using kernel data structures like "struct page" just to cast it back
    from char * to struct page * when it needs to handle highmem in
    zcache. Something seriously wrong is going on there in API terms so
    you can start by fixing that bit.

    > I hope the earlier explanation about frontswap_shrink helps.
    > It's also good to note that the only other successful Linux
    > implementation of swap compression is zram, and zram's
    > creator fully supports frontswap (https://lkml.org/lkml/2011/10/28/8)
    >
    > So where are we now? Are you now supportive of merging
    > frontswap? If not, can you suggest any concrete steps
    > that will gain your support?

    My problem is this is like zram, like mentioned it only solves the
    compression. There is no way it can store the compressed data on
    disk. And this is way more complex than zram, and it only makes the
    pooling size not fixed at swapon time... so very very small gain and
    huge complexity added (again compared to zram). zram in fact required
    absolutely zero changes to the VM. So it's hard to see how this is
    overall better than zram. If we deal with that amount of complexity we
    should at least be a little better than zram at runtime, while this is
    same.


    \
     
     \ /
      Last update: 2011-11-01 19:11    [W:8.348 / U:0.048 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site