Re: [patch 20/24] perfmon: system calls interface

[Thomas Gleixner]

On Wed, 26 Nov 2008, eranian@xxxxxxxxxxxxxx wrote:

> +asmlinkage long sys_pfm_write(int fd, int uflags,
> +			      int type,
> +			      void __user *ureq,
> +			      size_t sz)

> +asmlinkage long sys_pfm_read(int fd, int uflags,
> +			     int type,
> +			     void __user *ureq,
> +			     size_t sz)

After looking at both I did a diff of the two functions:

--- r.c	2008-11-27 14:27:54.000000000 +0100
+++ w.c	2008-11-27 14:27:52.000000000 +0100
@@ -36,10 +36,12 @@
 	ret = pfm_check_task_state(ctx, PFM_CMD_STOPPED, &flags);
 	if (ret)
 		goto skip;
-
 	switch(type) {
+	case PFM_RW_PMC:
+		ret = __pfm_write_pmcs(ctx, req, count);
+		break;
 	case PFM_RW_PMD:
-		ret = __pfm_read_pmds(ctx, req, count);
+		ret = __pfm_write_pmds(ctx, req, count);
 		break;
 	default:
 		PFM_DBG("invalid type=%d", type);
@@ -48,12 +50,13 @@
 skip:
 	spin_unlock_irqrestore(&ctx->lock, flags);
 
-	if (copy_to_user(ureq, req, sz))
-		ret = -EFAULT;
-
+	/*
+	 * This function may be on the critical path.
+	 * We want to avoid the branch if unecessary.
+	 */
 	if (fptr)
 		kfree(fptr);
 error:
 	pfm_release_ctx_from_fd(&cookie);
 	return ret;
 }

Both read and write are multiplexing syscalls already and 90% of the
code is the same.

	case PFM_RD_PMC:
 	case PFM_RD_PMD:
	case PFM_WR_PMC:
 	case PFM_WR_PMD:

would make them the same and safe a syscall and duplicated code.

Thanks,

	tglx

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