On Wed, Jan 19, 2011 at 06:10:39PM +0100, Peter Zijlstra wrote:
> > 19/21 mm-convert_i_mmap_lock_and_anon_vma-_lock_to_mutexes.patch
> > I suggest doing the anon_vma lock->mutex conversion separately here.
> > Acked-by: Hugh Dickins <hughd@xxxxxxxxxx>
> > except that in the past we have renamed a lock when we've done this
> > kind of conversion, so I'd expect anon_vma->mutex throughout now.
> > Or am I just out of date? I don't feel very strongly about it.
>
> Done.. however:
>
> Index: linux-2.6/include/linux/huge_mm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/huge_mm.h
> +++ linux-2.6/include/linux/huge_mm.h
> @@ -91,12 +91,8 @@ extern void __split_huge_page_pmd(struct
> #define wait_split_huge_page(__anon_vma, __pmd) \
> do { \
> pmd_t *____pmd = (__pmd); \
> - spin_unlock_wait(&(__anon_vma)->root->lock); \
> - /* \
> - * spin_unlock_wait() is just a loop in C and so the \
> - * CPU can reorder anything around it. \
> - */ \
> - smp_mb(); \
> + anon_vma_lock(__anon_vma); \
> + anon_vma_unlock(__anon_vma); \
> BUG_ON(pmd_trans_splitting(*____pmd) || \
> pmd_trans_huge(*____pmd)); \
> } while (0)
>
> Andrea, is that smp_mb() simply to avoid us doing anything before the
> lock is free? Why isn't there an mb() before to ensure nothing leaks
> past it from the other end?
The idea would be that by the time we read the pmd set to
pmd_trans_splitting with the page_table_lock hold, we're guaranteed
we'll see the anon-vma locked (if it's still locked). So it's ok if
spin_unlock_wait happens before reading the pmd_trans_splitting check
inside the CPU (all it matters is for it not to happen before the
spin_lock(&page_table_lock) which it can't by the acquire semantics of
the spinlock).
So in short we know we start with the anon_vma locked, and we just
wait as long as needed.
So we only need to protect to the stuff after spin_unlock_wait().
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
