Hi,
On Wed, 22 Dec 2010 10:40:58 +0200
Felipe Balbi <balbi@xxxxxx> wrote:
> We don't want to involve the entire stack on this test. It's better
> that you make a simple sysfs entry that will do something like:
>
> int count = 100000;
> u8 devctl;
>
> /* don't forget locking */
> while (--count)
> devctl = musb_readb(musb->regs, MUSB_DEVCTL);
>
> Then you use your function pointers and you can "emulate" the behavior
> with struct musb_io_ops with something like below:
Good, see the test result below.
> diff --git a/drivers/usb/musb/musb_core.c
> b/drivers/usb/musb/musb_core.c index 07cf394..a1682fc 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> Then you change the comment on musb_readb_show() to use the different
> versions and, maybe, a simple "time cat
> /sys/devices/platform/musb-omap2430/musb-hdrc/readb" will do to
> measure the difference.
Since the accuracy of 'time' utility is not good, I write a test case
based on your code posted and figure out time elapsed in kernel, follows
the test results:
test board: beagle xM, mpurate: 950
root@beagleboard:/sys/kernel/debug/musb# cat readw
direct musb_readw, count:1000000, time:223682us
single func deref musb_readw, count:1000000, time:236562us
dual func deref musb_readw, count:1000000, time:247016us
root@beagleboard:/sys/kernel/debug/musb# cat readw
direct musb_readw, count:1000000, time:223689us
single func deref musb_readw, count:1000000, time:237915us
dual func deref musb_readw, count:1000000, time:247150us
root@beagleboard:/sys/kernel/debug/musb# cat readw
direct musb_readw, count:1000000, time:223571us
single func deref musb_readw, count:1000000, time:237607us
dual func deref musb_readw, count:1000000, time:247088us
>From the above, single func deref access causes 5%~6% performance loss, and
dual func deref access causes ~10% performance loss compared with direct inline
function access. Also the test case should be more optimistic than actual running
case for cache footprint reason.
Secondly function deref produces larger code size both for caller and callee, see
the result in my last email.
So I suggest to avoid using function pointer as far as possible for better performance,
could you agree on it?
Below is the patch used in the test case above.
diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
index f948c94..366e685 100644
--- a/drivers/usb/musb/musb_debugfs.c
+++ b/drivers/usb/musb/musb_debugfs.c
@@ -250,6 +250,74 @@ static const struct file_operations musb_test_mode_fops = {
.release = single_release,
};
+#define TEST_CNT 1000000
+
+struct musb_io_ops {
+ u16 (*__musb_readw)(void __iomem *base, unsigned offset);
+};
+
+struct musb_io_ops the_op;
+
+struct musb_io_ops *op = &the_op;
+u16 (*__musb_readw)(const void __iomem *addr, unsigned offset);
+
+static void readw_bench(struct musb *musb, unsigned long count,
+ int type, unsigned long long *dur)
+{
+ unsigned short test;
+ unsigned long flags;
+ ktime_t calltime, delta, rettime;
+
+ spin_lock_irqsave(&musb->lock, flags);
+ calltime = ktime_get();
+ if (type == 0) {
+ while(count--)
+ test = musb_readw(musb->mregs, 0x6c);
+ } else if (type == 1) {
+ __musb_readw = musb_readw;
+ while(count--)
+ test = __musb_readw(musb->mregs, 0x6c);
+ } else if (type == 2) {
+ op->__musb_readw = musb_readw;
+ while(count--)
+ test = op->__musb_readw(musb->mregs, 0x6c);
+ }
+ rettime = ktime_get();
+ spin_unlock_irqrestore(&musb->lock, flags);
+
+ delta = ktime_sub(rettime, calltime);
+ *dur = (unsigned long long) ktime_to_ns(delta) >> 10;
+}
+
+static int musb_readw_show(struct seq_file *s, void *unused)
+{
+ struct musb *musb = s->private;
+ unsigned int count = TEST_CNT;
+ unsigned long long duration;
+
+ readw_bench(musb, count, 0, &duration);
+ seq_printf(s, "direct musb_readw, count:%d, time:%lldus\n", count, duration);
+
+ readw_bench(musb, count, 1, &duration);
+ seq_printf(s, "single func deref musb_readw, count:%d, time:%lldus\n", count, duration);
+
+ readw_bench(musb, count, 2, &duration);
+ seq_printf(s, "dual func deref musb_readw, count:%d, time:%lldus\n", count, duration);
+ return 0;
+}
+
+static int musb_readw_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, musb_readw_show, inode->i_private);
+}
+
+static const struct file_operations musb_readw_fops = {
+ .open = musb_readw_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
int __devinit musb_init_debugfs(struct musb *musb)
{
struct dentry *root;
@@ -276,6 +344,13 @@ int __devinit musb_init_debugfs(struct musb *musb)
goto err1;
}
+ file = debugfs_create_file("readw", S_IRUGO,
+ root, musb, &musb_readw_fops);
+ if (IS_ERR(file)) {
+ ret = PTR_ERR(file);
+ goto err1;
+ }
+
musb_debugfs_root = root;
return 0;
thanks,
--
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
