lkml.org 
[lkml]   [2012]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] fix bug introduced in "mm: simplify find_vma_prev()"
On Sun, Mar 4, 2012 at 4:52 PM, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
> This patch fixes a bug introduced in "mm: simplify find_vma_prev()". You
> can apply this, or alternatively revert the original patch.

Hmm.

I realize the patch is a bug-fix, but I do wonder if we shouldn't look
at alternatives.

In particular, how about we just change the rule to be clearer, and
make the rule be:

- no more find_vma_prev() AT ALL.

- callers get turned into "find_vma()" and then we have a
"vma_prev()", which basically does your thing.

- many of the callers seem to be interested in "prev" *only* if they
found a vma. For example, the do_mlock() kind of logic seems to be
common:

vma = find_vma_prev(current->mm, start, &prev);
if (!vma || vma->vm_start > start)
return -ENOMEM;

which implies that the current find_vma_prev() semantics are really
wrong, because they force us to do extra work in this case where we
really really don't care about the result.

So we could do an entirely mechanical conversion of

vma = find_vma_prev(mm, address, &prev_vma);

into

vma = find_vma(mm, address);
prev_vma = vma_prev(vma);

and then after we've done that conversion, it looks like the bulk of
them will not care about "prev_vma" if vma is NULL, so they can then
be replaced with a pure

prev_vma = vma->vm_prev;

instead. Leaving just the (few) cases that may care about the
"previous vma may be the last vma of the VM" case.

Hmm? And we'd get away from that horrible "find_vma_prev()" interface
that uses pointers to vma pointers for return values. Ugh. There only
seems to be nine callers in the whole kernel.

Linus


\
 
 \ /
  Last update: 2012-03-05 02:25    [W:0.063 / U:0.992 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site