Christoph Hellwig wrote:
> 
> ...
>  include/linux/mm.h |    7 -
>  mm/Makefile        |    2
>  mm/filemap.c       |  332 -----------------------------------------------------
>  mm/madvise.c       |  238 +++++++++++++++++++++++++++++++++++++
>  mm/mlock.c         |  158 ++++---------------------
>  mm/mmap.c          |   37 +++--
>  mm/mprotect.c      |  218 +++++++++++-----------------------
>  7 files changed, 365 insertions, 627 deletions

It's nice to keep the file movement and the functional changes
in separate patches.  Makes it easier to understand what has
been changed.   I have done that split.  (Took 30 seconds.  Do
that with bitkeeper ;))


 
>  static int mlock_fixup(struct vm_area_struct * vma,
>         unsigned long start, unsigned long end, unsigned int newflags)
>  {
> ...
> +
> +       spin_lock(&mm->page_table_lock);
> +       vma->vm_flags = newflags;
> +       spin_unlock(&mm->page_table_lock);

hrm.  What's being locked here?

> ...
> -static int splitvma(struct mm_struct *mm, struct vm_area_struct *mpnt, unsigned long addr)
> +int split_vma(struct mm_struct * mm, struct vm_area_struct * vma,
> +             unsigned long addr, int new_below)
> ...
>         spin_lock(&mm->page_table_lock);
> -       lock_vma_mappings(mpnt);
> +       lock_vma_mappings(vma);
>         __insert_vm_struct(mm, new);
> -       unlock_vma_mappings(mpnt);
> +       unlock_vma_mappings(vma);
>         spin_unlock(&mm->page_table_lock);

OK, split_vma() has the same rank ordering bug as vma_link().  i_shared_lock
should nest outside page_table_lock.  That's how vmtruncate does it, and
it seems to be a logical ranking to me.

I don't know why vma_link() takes page_table_lock at all.  But then, I
don't know what page_table_lock actually locks, apart from page tables.
It's all very weird.

Could you please fix up this, fix up vma_link, do a little audit of
the other paths which play with these locks and update the locking comment
in filemap.c?

Generally: I'm going to need a lot of help understanding what this
patch does, and why it does it.  Could you prepare a little description
of that?

I've attached the separated diffs here - I'll do a 2.5.33-mm1 today,
including these diffs.  That should make syncing up easier.