Re: sparsemem panic in 2.6.17-rc5-mm1 and -mm2

[Andy Whitcroft]

Andrew Morton wrote:
> On Wed, 7 Jun 2006 09:29:50 -0700
> Andrew Morton <akpm@xxxxxxxx> wrote:
> 
> 
>>Note that the code can be optimised:
>>
>>	if (page_zone_id(page) != page_zone_id(buddy))
>>
>>...
>>
>>static inline int page_zone_id(struct page *page)
>>{
>>	return (page->flags >> ZONETABLE_PGSHIFT) & ZONETABLE_MASK;
>>}
>>
>>We don't need to perform the shift to make that comparison.  If the
>>compiler's sufficiently smart it will be able to optimise that for us.
>>
>><checks>
>>
>>        shrl    $30, %edx       #, <variable>.flags
>>        shrl    $30, %eax       #, <variable>.flags
>>        cmpl    %eax, %edx      # <variable>.flags, <variable>.flags
>>
>>Nope, not smart enough.
> 
> 
> I take it back:
> 
> .LFB856:
> 	.loc 1 314 0
> .LVL540:
> 	pushl	%ebp	#
> .LCFI419:
> 	movl	%esp, %ebp	#,
> .LCFI420:
> 	pushl	%ebx	#
> .LCFI421:
> 	.loc 1 314 0
> 	movl	%edx, %ebx	# buddy, buddy
> 	.loc 1 320 0
> 	movl	(%eax), %edx	# <variable>.flags, <variable>.flags
> .LVL541:
> 	movl	(%ebx), %eax	# <variable>.flags, <variable>.flags
> .LVL542:
> 	shrl	$30, %edx	#, <variable>.flags
> 	shrl	$30, %eax	#, <variable>.flags
> 	cmpl	%eax, %edx	# <variable>.flags, <variable>.flags
> 	jne	.L587	#,
> .LBB1082:
> 
> The compiler's done something sneaky there and has omitted the masking.
> 
> 
> Anyway.  It sure doesn't look like it's worth a config option.

Yep I forgot about that.  We fought hard to keep the NODEZONE zonetable
index at the very edge of the flags for this very reason.  The
optimisation is worth taking care to order the fields in this area.

So if we assume barrel roll and a mask are equivalent cost to the
processor (fair I'd say) this comparison is as optimal as it can be.

I'll spin and test just an always on version and send you that to
replace the other patches.

-apw
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/