linux-kernel - Re: [PATCH v2] clocksource/drivers/dw_apb_timer: Use {readl|writel}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5804676.futR40KtNg@wuerfel>
Date:	Fri, 13 Nov 2015 14:51:57 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Jisheng Zhang <jszhang@...vell.com>
Cc:	daniel.lezcano@...aro.org, tglx@...utronix.de,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2] clocksource/drivers/dw_apb_timer: Use {readl|writel}_relaxed

On Friday 13 November 2015 20:57:27 Jisheng Zhang wrote:
> 
> > that the conversion is correct. You could introduce apbt_readl_releaxed()
> > etc functions and call them from __apbt_read_clocksource()
> > and apbt_next_event().
> 
> I'm not sure whether such changes would make the code a bit complex. From
> another side, it's safe to always use relaxed version in this driver, so
> is it better to switch to relaxed version no matter the code path benefit from
> it or not?

We've had problems in the past when people blindly converted whole drivers,
so I try to discourage that in general, if only to get people to pay more
attention when copying from one driver to another.

> PS: for the global timer related patch, I just hack code a bit to make it works
> as clockevent on my platform, and I still try to think about a test case to
> measure the improvement, cyclictest? Any idea is appreciated.

Measuring performance of timers is tricky by definition, but you could try
to sample the amount of time you spend setting up timers like this

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index a2cb6fae9295..da88347718ae 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -95,6 +95,8 @@ static u64 gt_counter_read(void)
 static void gt_compare_set(unsigned long delta, int periodic)
 {
 	u64 counter = gt_counter_read();
+	static u64 total_time;
+	static u32 count;
 	unsigned long ctrl;
 
 	counter += delta;
@@ -110,6 +112,12 @@ static void gt_compare_set(unsigned long delta, int periodic)
 
 	ctrl |= GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE;
 	writel(ctrl, gt_base + GT_CONTROL);
+
+	total_time += gt_counter_read() - counter;
+	count++;
+
+	if (((count - 1) & 0xfff) == 0xfff)
+		printk(KERN_INFO "gt_compare_set time %lld\n", total_time / (count >> 12));
 }
 
 static int gt_clockevent_shutdown(struct clock_event_device *evt)

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/