lkml.org 
[lkml]   [2008]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC][PATCH 9/7] mm: fix mm_take_all_locks() locking order
    On Thu, 7 Aug 2008, Peter Zijlstra wrote:
    >
    > Which the locking hierarchy in mm/rmap.c confirms as 'valid'.
    >
    > Although I don't think there are any users of these two locks that don't
    > hold the mmap_sem, therefore the nesting is strictly ok, but since we
    > already have an established order, we might as well respect it.

    Yes, I agree.

    > Fix this by first taking all the mapping->i_mmap_lock instances and then
    > take all anon_vma->lock instances.

    Okay. I'd have preferred taking anon_vma lock after i_mmap_lock
    each time around the loop, but imagine that's just as problematic
    for lockdep as the original.

    >
    > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

    Acked-by: Hugh Dickins <hugh@veritas.com>

    > ---
    > mm/mmap.c | 10 ++++++++--
    > 1 file changed, 8 insertions(+), 2 deletions(-)
    >
    > Index: linux-2.6/mm/mmap.c
    > ===================================================================
    > --- linux-2.6.orig/mm/mmap.c
    > +++ linux-2.6/mm/mmap.c
    > @@ -2358,11 +2358,17 @@ int mm_take_all_locks(struct mm_struct *
    > for (vma = mm->mmap; vma; vma = vma->vm_next) {
    > if (signal_pending(current))
    > goto out_unlock;
    > - if (vma->anon_vma)
    > - vm_lock_anon_vma(mm, vma->anon_vma);
    > if (vma->vm_file && vma->vm_file->f_mapping)
    > vm_lock_mapping(mm, vma->vm_file->f_mapping);
    > }
    > +
    > + for (vma = mm->mmap; vma; vma = vma->vm_next) {
    > + if (signal_pending(current))
    > + goto out_unlock;
    > + if (vma->anon_vma)
    > + vm_lock_anon_vma(mm, vma->anon_vma);
    > + }
    > +
    > ret = 0;
    >
    > out_unlock:


    \
     
     \ /
      Last update: 2008-08-07 14:53    [W:0.020 / U:30.640 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site