From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752849AbcFNRqj (ORCPT <rfc822;w@1wt.eu>);
	Tue, 14 Jun 2016 13:46:39 -0400
Received: from mail-qk0-f169.google.com ([209.85.220.169]:35906 "EHLO
	mail-qk0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752112AbcFNRqi (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Tue, 14 Jun 2016 13:46:38 -0400
Date: Tue, 14 Jun 2016 13:46:34 -0400 (EDT)
From: Nicolas Pitre <nicolas.pitre@linaro.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
cc: tglx@linutronix.de, shreyas@linux.vnet.ibm.com,
        linux-kernel@vger.kernel.org, peterz@infradead.org, rafael@kernel.org,
        vincent.guittot@linaro.org
Subject: Re: [PATCH V5] irq: Track the interrupt timings
In-Reply-To: <1465922036-27100-1-git-send-email-daniel.lezcano@linaro.org>
Message-ID: <alpine.LFD.2.20.1606141333260.1714@knanqh.ubzr>
References: <1465922036-27100-1-git-send-email-daniel.lezcano@linaro.org>
User-Agent: Alpine 2.20 (LFD 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, 14 Jun 2016, Daniel Lezcano wrote:

[...]

> +void __handle_timings(struct irq_desc *desc)
> +{
> +	struct irq_timings *timings;
> +	u64 prev, now, diff;
> +
> +	timings = this_cpu_ptr(desc->timings);
> +	now = local_clock();
> +	prev = timings->timestamp;
> +	timings->timestamp = now;
> +
> +	/*
> +	 * In case it is the first time this function is called, the
> +	 * 'prev' variable will be zero which reflects the time origin
> +	 * when the system booted.
> +	 */
> +	diff = now - prev;
> +
> +	/* The oldest value corresponds to the next index. */
> +	timings->w_index = (timings->w_index + 1) & IRQ_TIMINGS_MASK;
> +	timings->values[timings->w_index] = diff;
> +}

What about simply this:

void __handle_timings(struct irq_desc *desc)
{
	struct irq_timings *timings = this_cpu_ptr(desc->timings);
	timings->w_index = (timings->w_index + 1) & IRQ_TIMINGS_MASK;
	timings->values[timings->w_index] = local_clock();
}

?

Then you could s/__handle_timings/__record_irq_time/ to better represent 
what it does.  And both the difference and the summing of squares could 
be done upon entering idle instead.


Nicolas