Re: dtrace and ZFS update

[Chuck Silvers <chuq%chuq.com@localhost>]

On Sat, Oct 14, 2017 at 01:16:52AM +0000, maya%netbsd.org@localhost wrote:
> More of my unprofessional opinion since we'll need all the help we can
> get...
> 
> +void *
> +dtrace_casptr(volatile void *target, volatile void *cmp, volatile void *new)
> +*/
> +EENTRY(dtrace_casptr)
> +ENTRY(dtrace_cas32)
> +#if __ARM_ARCH >= 6
> +
> +1:	ldrex	r3, [r0]	/* Load target */
> +	cmp	r3, r1		/* Check if *target == cmp */
> +	bne	2f		/* No, return */
> +	strex	ip, r2, [r0]	/* Store new to target */
> +	cmp	ip, #0		/* Did the store succeed? */
> +	bne	1b		/* No, try again */
> +2:	mov	r0, r3		/* Return the value loaded from target */
> +	RET
> +
> 
> ..
> 
> Surely we have a function like this already, does dtrace really
> need its own asm implementation>?

dtrace has separate copies of a number of functions that are used during
the execution of a probe, so that the original function can be probed
without creating an infinite loop.

the __ARM_ARCH >= 6 version of this code is unchanged from upstream,
and the other version isn't likely to change ever, so I don't think
this is worth doing differently.


> +static int
> +log2(int size)
> +{
> +	switch (size) {
> +	case 1: return (0);
> +	case 2: return (1);
> +	case 4: return (2);
> +	case 8: return (3);
> +	}
> +	return (0);
> +}
> 
> Using the name of a standard C function differently sounds dangerous


this function is static so it only affects this one file.  this file is
very unlikely to ever want to use the libm log2() that operates on doubles,
so it doesn't seem worth diverging from upstream for this.


in general I agree with you about both of these issues, but I want to take
a pretty firm stance about not diverging from upstream for the CDDL code
unless it's really necessary.

-Chuck