[patch 10/11] spufs: new entries for SPU special purpose registers.

[Arnd Bergmann]

On Dunnersdag 22 September 2005 20:44, Milton Miller wrote:

> Another style that is frequently used throught out the kernel and often
> results in smaller code gen is to set ret and goto the exit path.  The
> compiler can then reuse the function call code.
> 
> Something like this  (untested):
> 
> +static ssize_t
> +spufs_fpcr_read(struct file *file, char __user * buffer,
> +		size_t size, loff_t * pos)
> +{
> +	struct spu_context *ctx = file->private_data;
> +	struct spu_lscsa *lscsa = ctx->csa.lscsa;
> +	int ret;
> +
> +	spu_acquire_saved(ctx);
> +	if (ctx->state == SPU_STATE_LOCKED) {
> +		ret = -EAGAIN;
> + 		goto release;
> +	}
> +
> +	ret = simple_read_from_buffer(buffer, size, pos,
> +				      &lscsa->fpcr, sizeof(lscsa->fpcr));
> +
> + release:
> +	spu_release(ctx);
> +	return ret;
> +}
> 
> A further changed use less pervasively puts the setting of ret before
> the if, so the body is a simple goto.   I haven't looked at how the 
> resulting code compares.

Yes, you're absolutely right here. I usually prefer that style as well
and it is a trivial change.

> Also, do we expect the state to be locked?  Maybe that should be made
> unlikely().  That may also improve the resulting code.

I don't like to put unlikely()/likely() into places that are not
performance critical. This function is only called for debugging
purposes and _very_ slow already because of the spu_acquire_saved
overhead, so I'd rather go for simplicity.

Thinking about this some more, I'm also not sure if -EAGAIN makes
sense in the first place. SPU_STATE_LOCKED is only used if we can't
use sparsemem for dynamic mmaps of /spu/*/mem, and in that case 
accessing fpcr or regs will not work again on the same spu. Maybe
-EINVAL is better here.

	Arnd <><