[patch 0/6] [RFC] MMU Notifiers V3 - Kernel

This is a discussion on [patch 0/6] [RFC] MMU Notifiers V3 - Kernel ; On Wed, Jan 30, 2008 at 03:55:37PM -0800, Christoph Lameter wrote: > On Thu, 31 Jan 2008, Andrea Arcangeli wrote: > > > > I think Andrea's original concept of the lock in the mmu_notifier_head > > > structure was ...

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2
Results 21 to 22 of 22

Thread: [patch 0/6] [RFC] MMU Notifiers V3

  1. Re: [kvm-devel] [patch 1/6] mmu_notifier: Core code

    On Wed, Jan 30, 2008 at 03:55:37PM -0800, Christoph Lameter wrote:
    > On Thu, 31 Jan 2008, Andrea Arcangeli wrote:
    >
    > > > I think Andrea's original concept of the lock in the mmu_notifier_head
    > > > structure was the best. I agree with him that it should be a spinlock
    > > > instead of the rw_lock.

    > >
    > > BTW, I don't see the scalability concern with huge number of tasks:
    > > the lock is still in the mm, down_write(mm->mmap_sem); oneinstruction;
    > > up_write(mm->mmap_sem) is always going to scale worse than
    > > spin_lock(mm->somethingelse); oneinstruction;
    > > spin_unlock(mm->somethinglese).

    >
    > If we put it elsewhere in the mm then we increase the size of the memory
    > used in the mm_struct.


    Yes, and it will increase of the same amount of RAM that you pretend
    everyone to pay even if MMU_NOTIFIER=n after your patch is applied (vs
    mine that generated 0 ram utilization increase when
    MMU_NOTIFIER=n). And the additional ram will provide not just
    self-contained locking but higher scalability too.

    I think it's much more important to generate zero ram and CPU overhead
    for the embedded (this is something I was very careful to enforce in
    all my patches), than to reduce scalability and not having a self
    contained locking on full configurations with MMU_NOTIFIER=y.

    > Hmmmm.. exit_mmap is only called when the last reference is removed
    > against the mm right? So no tasks are running anymore. No pages are left.
    > Do we need to serialize at all for mmu_notifier_release?


    KVM sure doesn't need any locking there. I thought somebody had to
    possibly take a pin on the "mm_count" and pretend to call
    mmu_notifier_register at will until mmdrop was finally called, in a
    out of order fashion given mmu_notifier_release was implemented like
    if the list could change from under it. Note mmdrop != mmput. mmput
    and in turn mm_users is the serialization point if you prefer to drop
    all locking from _release. Nobody must ever attempt a mmu_notifier_*
    after calling mmput for that mm. That should be enough to be
    safe. I'm fine either ways...
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [kvm-devel] [patch 1/6] mmu_notifier: Core code

    On Thu, 31 Jan 2008, Andrea Arcangeli wrote:

    > > Hmmmm.. exit_mmap is only called when the last reference is removed
    > > against the mm right? So no tasks are running anymore. No pages are left.
    > > Do we need to serialize at all for mmu_notifier_release?

    >
    > KVM sure doesn't need any locking there. I thought somebody had to
    > possibly take a pin on the "mm_count" and pretend to call
    > mmu_notifier_register at will until mmdrop was finally called, in a
    > out of order fashion given mmu_notifier_release was implemented like
    > if the list could change from under it. Note mmdrop != mmput. mmput
    > and in turn mm_users is the serialization point if you prefer to drop
    > all locking from _release. Nobody must ever attempt a mmu_notifier_*
    > after calling mmput for that mm. That should be enough to be
    > safe. I'm fine either ways...


    exit_mmap (where we call invalidate_all() and release()) is called when
    mm_users == 0:

    void mmput(struct mm_struct *mm)
    {
    might_sleep();

    if (atomic_dec_and_test(&mm->mm_users)) {
    exit_aio(mm);
    exit_mmap(mm);
    if (!list_empty(&mm->mmlist)) {
    spin_lock(&mmlist_lock);
    list_del(&mm->mmlist);
    spin_unlock(&mmlist_lock);
    }
    put_swap_token(mm);
    mmdrop(mm);
    }
    }
    EXPORT_SYMBOL_GPL(mmput);

    So there is only a single thread executing at the time when
    invalidate_all() is called from exit_mmap(). Then we drop the
    pages, and the page tables. After the page tables we call the ->release
    method and then remove the vmas.

    So even dropping off the mmu_notifier chain in invalidate_all() could be
    done without an issue and without locking.

    Trouble is if other callbacks attempt the same. Do we need to support the
    removal from the mmu_notifier list in invalidate_range()?

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 2 of 2 FirstFirst 1 2