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

[Milton Miller]

On Sep 16, 2005, at 5:01 PM, Arnd Bergmann wrote:

> This patch implements SPUFS directory entries for various special
> purpose registers.  These include the SPU floating point control
> register (fpcr), decrementer (decr), and machine status register
> (srr0).  It should now be possible to query/set all user state
> from SPUFS, implement get/set_context interfaces, etc.
>
> From: Mark Nutter <mnutter at us.ibm.com>
> Signed-off-by: Arnd Bergmann <arndb at de.ibm.com>
>
> --
>
>  file.c |  170 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 170 insertions(+)
>
> Index: linux-cg/fs/spufs/file.c
> ===================================================================
> --- linux-cg.orig/fs/spufs/file.c
> +++ linux-cg/fs/spufs/file.c
> @@ -223,6 +223,60 @@ static struct file_operations spufs_regs
>  	.llseek  = generic_file_llseek,
>  };
>
> +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) {
> +		spu_release(ctx);
> +		return -EAGAIN;
> +	}
> +
> +	ret = simple_read_from_buffer(buffer, size, pos,
> +				      &lscsa->fpcr, sizeof(lscsa->fpcr));
> +
> +	spu_release(ctx);
> +	return ret;
> +}
>

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.

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

milton