Re: [Xen-devel] [PATCH] fix page_list_splice()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.06.12 at 11:02, Keir Fraser <keir@xxxxxxx> wrote:
> On 06/06/2012 09:23, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> Other than in __list_splice(), the first element's prev pointer doesn't
>> need adjustment here - it already is PAGE_LIST_NULL. Rather than fixing
>> the assignment (to formally match __list_splice()), simply assert that
>> this assignment is really unnecessary.
>>
>> Reported-by: Jisoo Yang <jisooy@xxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> 
>> --- a/xen/include/xen/mm.h
>> +++ b/xen/include/xen/mm.h
>> @@ -270,7 +270,8 @@ page_list_splice(struct page_list_head *
>>      last = list->tail;
>>      at = head->next;
>>  
>> -    first->list.prev = page_to_pdx(head->next);
>> +    /* Both first->list.prev and at->list.prev are PAGE_LIST_NULL. */
> 
> ASSERT(first->list.prev == PAGE_LIST_NULL); ?
> 
> It seems odd to have one assumption encoded as an ASSERTion, and one as a
> code comment... A second assertion makes the assumption explicit, and
> run-time checked in debug builds.

But the __list_splice() equivalent assignment would be

    first->list.prev = at->list.prev;

which is why I chose the assert expression that way, yet made
the comment clarify what the actual state is. If the comment
just repeated what the ASSERT() already says, I'd rather drop
the comment altogether.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel