Re: Linux 2.6.32-rc1

[Eric Dumazet]

Linus Torvalds a écrit :
> 
> On Wed, 30 Sep 2009, Arjan van de Ven wrote:
>> +ENTRY(cmpxchg8b_emu)
>> +	CFI_STARTPROC
>> +
>> +	push %edi
>> +	push %ebx
>> +	push %ecx
>> +	/* disable interrupts */
>> +	pushf
>> +	pop %edi
>> +	cli
>> +
>> +	cmpl %edx, 4(%esi)
>> +	jne 1f
>> +	cmpl %eax, (%esi)
>> +	jne 1f
>> +
>> +	xchg (%esi), %ebx
>> +	xchg 4(%esi), %ecx
>> +	mov %ebx, %eax
>> +	mov %ecx, %edx
> 
> Ok, so why do you do this? You've just checked that the 8 bytes at esi are 
> the same as edx:eax, why do you do that odd xchg?
> 
> Why isn't this part just
> 
> 	mov %ebx,(%esi)
> 	mov %ecx,4(%esi)
> 
> and leave ebx/ecx alone?
> 
> I also don't see why you play games with eflags and %edi. Just leave the 
> flags on the stack. So it all would look like
> 
> 	#
> 	# Emulate 'cmpxchg8b (%esi)' except we don't
> 	# set the whole ZF thing (caller will just
> 	# compare eax:edx with the expected value)
> 	#
> 	cmpxchg8b_emu:
> 		pushfl
> 		cli
> 		cmpl (%esi),%eax
> 		jne not_same
> 		cmpl 4(%esi),%edx
> 		jne not_same
> 		movl %ebx,(%esi)
> 		movl %ecx,4(%esi)
> 		popfl
> 		ret
> 	not_same:
> 		movl (%esi),%eax
> 		movl 4(%esi),%edx
> 		popfl
> 		ret
> 
> I dunno. You _could_ use edi/ebp as a temporary for 'dest' to only do the 
> load once (and add push/pop to save the registers), but it's not like the 
> above is really atomic anyway, and it very much depends on 'cli' just 
> making sure that nothing else happens. So the above seems to be the 
> simplest emulation, considering the interfaces..
> 
> UNTESTED! I wrote this in an email editor. I haven't assembled it or 
> verified the semantics or workingness in any way, shape or form.
> 
> 			Linus

Yes, I provided following version in my first answer but apparently it was too complex :)

ENTRY(cmpxchg8b_emu)
	CFI_STARTPROC

	/* disable interrupts */
	pushf
	cli

	cmpl  %eax,(%esi)
	jne   1f
	cmpl  %edx,4(%esi)
	jne   2f

	mov   %ebx,(%esi)
	mov   %ecx,4(%esi)

	/* restore interrupts */
	popf
	ret
1:
	mov (%esi), %eax
2:
	mov 4(%esi), %edx
	popf
	ret
	CFI_ENDPROC
ENDPROC(cmpxchg8b_emu)

--
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/