linux-kernel - Re: [PATCH]: tick-sched.c build fix

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20070226.112239.112290727.davem@davemloft.net>
Date:	Mon, 26 Feb 2007 11:22:39 -0800 (PST)
From:	David Miller <davem@...emloft.net>
To:	tglx@...utronix.de
Cc:	johnstul@...ibm.com, linux-kernel@...r.kernel.org,
	peter.keilty@...com
Subject: Re: [PATCH]: tick-sched.c build fix

From: Thomas Gleixner <tglx@...utronix.de>
Date: Sun, 25 Feb 2007 10:04:41 +0100

> On Sat, 2007-02-24 at 21:42 -0800, David Miller wrote:
> > While getting dynticks/hrtimers up on sparc64 I ran into
> > the following build problem.  If you use get_irq_regs() you
> > need to include asm/irq_regs.h
> > 
> > Signed-off-by: David S. Miller <davem@...emloft.net>
> 
> Acked-by: Thomas Gleixner <tglx@...utronix.de>

Thanks for reviewing Thomas, I'll push the two bug fixes I
posted, with your ACKs, to Linus.

Two things remaining:

1) The change of clocksource_done_booting() to an fs_initcall()
   really is necessary.  Could you please push that to Linus?

2) I got bit by shifting problems (again) on sparc64.

Let me elaborate about #2.  Nobody writing these timer drivers
should be concerned by all of these details, I think, they will
get it wrong just as I did.

In my case a shift of 32 was too high for TSC based clockevent
on Niagara.

What we really should do is provide a clockevent_init_multshift()
or whatever that takes ticks_per_sec as an argument and this
function figured out an appropriate shift and multiplier to use.
It could be as simple as:

void clockevent_set_multshift(struct clock_event_device *edev, unsigned long hz)
{
	u64 mult;
	int shift = 32;

	while (1) {
		mult = div_sc(hz, NSEC_PER_SEC, shift);
		if (mult && (mult >> 32UL) == 0UL)
			break;

		shift--;
	}

	edev->shift = shift;
	edev->mult = (u32) mult;
}

With this, nobody will get bit by problems again.  We should
provide something similar, if not identical, for clocksources
too.  Perhaps we could even start trying at initial shift values
larger than 32.

What do you think?

Another thing I noticed is that clock_event_device->mult is
declared as an unsigned long, but it is clearly limited to
being a u32 as it is used as the second argument to do_div().

BTW, that's how I hit the Niagara shift issue, divide by zero
in clockevent_delta2ns() :-)
-
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/