* Hugh Dickins <[EMAIL PROTECTED]> [2006-12-27 01:04]:
> > > The first message at http://bugs.debian.org/394392 contains some
> > > information about it, but I'm sure Jeff Licquia (CCed) can provide
> > > more information if necessary.
> > 
> How quickly I forget!  It wasn't until I looked into the git history
> that I remembered how I'd discovered this already, just before 2.6.17
> went out; but thought myself likely to introduce a more serious bug
> if I tried to fix it in a hurry.  (Then forgot to send in a fix for
> 2.6.18, expecting Linus to put in Peter's more significant mods.)
> 
> So beware of the fix below, which I'll confess to not even having
> tested in practice: please review carefully, it's a confusing loop
> (rather like the mincore one which Linus has just cleaned up in
> 2.6.20-rc); and it took me a while to decide which of our many
> versions of that loop to start from in fixing it - chose to
> rely on what I'd worked out earlier when redoing it for 2.6.19.
> 
> Material for -stable?  Well, my judgement was, and is, not really:
> the 2.6.17/2.6.18 behaviour isn't quite right, but though it doesn't
> match the man page and that testsuite, what it does isn't actually
> harmful at all.  If this patch is more acceptable to Debian if it
> does appear in -stable, I can try sending it to Chris and Greg:
> but I won't be able to argue for it very forcefully.  Maybe better
> for you to consider it as just a subpatch of what went into 2.6.19,
> which I was wrong to have mixed in with Peter's work.
> 
> I certainly think that this patch below (if it satisfies your review
> and testing) is much more suitable for Debian's 2.6.18 than Peter's
> suite of patches.  Not to put those down at all: the reverse, they're
> a significant contribution to 2.6.19, which just wouldn't be expected
> in any 2.6.18 kernel (and sadly, as you've found, are exposing still
> unexplained problems there).  Anyway, the patch and its comment...

I'm CCing the debian-lsb list.  Can someone please apply this patch to
2.6.18 and see whether the msync problem from #394392 is gone?  I've
also applied it to our kernel SVN repository.


From: Hugh Dickins <[EMAIL PROTECTED]>

Fix 2.6.18 (or 2.6.17) sys_msync to report -ENOMEM as before when an
unmapped area falls within its range, and not to overshoot: to satisfy
LSB 3.1 tests and to fix Debian Bug#394392.  Took the 2.6.19 sys_msync
as starting point (including its cleanup of repeated "current->mm"s),
reintroducing the msync_interval and balance_dirty_pages_ratelimited_nr
previously needed.

Signed-off-by: Hugh Dickins <[EMAIL PROTECTED]>
---

 mm/msync.c |   66 +++++++++++++++++++++++++++----------------------------------
 1 file changed, 30 insertions(+), 36 deletions(-)

--- 2.6.18/mm/msync.c   2006-09-20 04:42:06.000000000 +0100
+++ linux/mm/msync.c    2006-12-26 23:52:58.000000000 +0000
@@ -146,10 +146,10 @@ static int msync_interval(struct vm_area
 asmlinkage long sys_msync(unsigned long start, size_t len, int flags)
 {
        unsigned long end;
+       struct mm_struct *mm = current->mm;
        struct vm_area_struct *vma;
        int unmapped_error = 0;
        int error = -EINVAL;
-       int done = 0;

        if (flags & ~(MS_ASYNC | MS_INVALIDATE | MS_SYNC))
                goto out;
@@ -169,64 +169,58 @@ asmlinkage long sys_msync(unsigned long 
         * If the interval [start,end) covers some unmapped address ranges,
         * just ignore them, but return -ENOMEM at the end.
         */
-       down_read(&current->mm->mmap_sem);
-       vma = find_vma(current->mm, start);
-       if (!vma) {
-               error = -ENOMEM;
-               goto out_unlock;
-       }
-       do {
+       down_read(&mm->mmap_sem);
+       vma = find_vma(mm, start);
+       for (;;) {
                unsigned long nr_pages_dirtied = 0;
                struct file *file;
 
+               /* Still start < end. */
+               error = -ENOMEM;
+               if (!vma)
+                       goto out_unlock;
                /* Here start < vma->vm_end. */
                if (start < vma->vm_start) {
-                       unmapped_error = -ENOMEM;
                        start = vma->vm_start;
-               }
-               /* Here vma->vm_start <= start < vma->vm_end. */
-               if (end <= vma->vm_end) {
-                       if (start < end) {
-                               error = msync_interval(vma, start, end, flags,
-                                                       &nr_pages_dirtied);
-                               if (error)
-                                       goto out_unlock;
-                       }
-                       error = unmapped_error;
-                       done = 1;
-               } else {
-                       /* Here vma->vm_start <= start < vma->vm_end < end. */
-                       error = msync_interval(vma, start, vma->vm_end, flags,
-                                               &nr_pages_dirtied);
-                       if (error)
+                       if (start >= end)
                                goto out_unlock;
+                       unmapped_error = -ENOMEM;
                }
+               /* Here vma->vm_start <= start < vma->vm_end. */
+               error = msync_interval(vma, start, min(end, vma->vm_end),
+                                               flags, &nr_pages_dirtied);
+               if (error)
+                       goto out_unlock;
                file = vma->vm_file;
                start = vma->vm_end;
                if ((flags & MS_ASYNC) && file && nr_pages_dirtied) {
                        get_file(file);
-                       up_read(&current->mm->mmap_sem);
+                       up_read(&mm->mmap_sem);
                        balance_dirty_pages_ratelimited_nr(file->f_mapping,
                                                        nr_pages_dirtied);
                        fput(file);
-                       down_read(&current->mm->mmap_sem);
-                       vma = find_vma(current->mm, start);
+                       if (start >= end)
+                               goto out;
+                       down_read(&mm->mmap_sem);
+                       vma = find_vma(mm, start);
                } else if ((flags & MS_SYNC) && file &&
                                (vma->vm_flags & VM_SHARED)) {
                        get_file(file);
-                       up_read(&current->mm->mmap_sem);
+                       up_read(&mm->mmap_sem);
                        error = do_fsync(file, 0);
                        fput(file);
-                       down_read(&current->mm->mmap_sem);
-                       if (error)
-                               goto out_unlock;
-                       vma = find_vma(current->mm, start);
+                       if (error || start >= end)
+                               goto out;
+                       down_read(&mm->mmap_sem);
+                       vma = find_vma(mm, start);
                } else {
+                       if (start >= end)
+                               goto out_unlock;
                        vma = vma->vm_next;
                }
-       } while (vma && !done);
+       }
 out_unlock:
-       up_read(&current->mm->mmap_sem);
+       up_read(&mm->mmap_sem);
 out:
-       return error;
+       return error ? : unmapped_error;
 }


-- 
Martin Michlmayr
http://www.cyrius.com/


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to