svn commit: r248185 - user/attilio/vmcontention/sys/vm

[Attilio Rao]

On Tue, Mar 12, 2013 at 5:26 PM, Alan Cox <alc at rice.edu> wrote:
> On 03/12/2013 07:04, Attilio Rao wrote:
>> On Tue, Mar 12, 2013 at 7:14 AM, Alan Cox <alc at freebsd.org> wrote:
>>> Author: alc
>>> Date: Tue Mar 12 06:14:31 2013
>>> New Revision: 248185
>>> URL: http://svnweb.freebsd.org/changeset/base/248185
>>>
>>> Log:
>>>   When transferring the page from one object to another, don't insert the
>>>   page into its new object until the page's pindex has been updated.
>>>   Otherwise, one code path within vm_radix_insert() may use the wrong
>>>   pindex value.
>> This is just a style change really because the code already subtracts
>> offindxstart from current m->pindex when inserting. The pindex on the
>> page is then adjusted when the real subtraction happens.
>> IIRC, I did this way because of less diffs against -CURRENT, but the
>> code looks correct to me already in the older version.
>>
>
> No, as I say in the commit message, there is a path in vm_radix_insert()
> that uses "page->pindex" and not the "index" argument:
>
>         /*
>          * A new node is needed because the right insertion level is
> reached.
>          * Setup the new intermediate node and add the 2 children: the
>          * new object and the older edge.
>          */
>         tmp2 = vm_radix_node_get(vm_radix_trimkey(page->pindex, clev -
> 1), 2,
>             clev);
>
> Prior to this change, if we follow this path, then we are using the
> wrong "pindex" value at this point in vm_radix_insert().

My bad, that should use "index" as it was supposed to be, not the page
one. I likely added it as a cleanup after having reworked all the
other functions, introducing the bug.
It is courious it never showed up.
I'd rather fix vm_radix_insert() to not consider the page index. This
is a step forward anyway in generalizing the code.

Attilio


-- 
Peace can only be achieved by understanding - A. Einstein