linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: "zhaoyan.liao" <zhaoyan.liao@linux.alibaba.com>,
	mingo@redhat.com, hpa@zytor.com, dwmw@amazon.co.uk
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	songmuchun@bytedance.com, likunkun@bytedance.com,
	guancheng.rjk@alibaba-inc.com,
	"zhaoyan.liao" <zhaoyan.liao@linux.alibaba.com>
Subject: Re: [PATCH] use 64bit timer for hpet
Date: Wed, 07 Jul 2021 12:04:11 +0200	[thread overview]
Message-ID: <875yxmqw2s.ffs@nanos.tec.linutronix.de> (raw)
In-Reply-To: <1625213625-25745-1-git-send-email-zhaoyan.liao@linux.alibaba.com>

Liao,

On Fri, Jul 02 2021 at 16:13, zhaoyan liao wrote:
> The kernel judges whether the tsc clock is accurate in the
> clocksource_watchdog background thread function. The hpet clock source
> is 32-bit, but tsc is 64-bit. Therefore, when the system is busy and the
> clocksource_watchdog cannot be scheduled in time, the hpet clock may
> overflow and cause the system to misjudge tsc as unreliable.

Seriously? The wrap-around time for 32bit HPET @24MHz is ~3 minutes.

> In this case, we recommend that the kernel adopts the 64-bit hpet clock
> by default to keep the width of the two clock sources the same to reduce
> misjudgment. Some CPU models may not support 64-bit hpet, but according
> to the description of the CPU's register manual, it does not affect our
> reading action.

So much for the theory.

> -#define HPET_MASK			CLOCKSOURCE_MASK(32)
> +#define HPET_MASK			CLOCKSOURCE_MASK(64)

How is that valid for a 32bit HPET? This breaks the clocksource.
 
> +inline unsigned long hpet_readq(unsigned int a)
> +{
> +	return readq(hpet_virt_address + a);

Breaks 32bit build immediately.

Aside of that the reason why the kernel does not support 64bit HPET is
that there are HPETs which advertise 64bit support, but the
implementation is buggy.

IOW, while this works for your hardware this breaks quite some parts of
the universe. Not really a good approach.

Thanks,

        tglx

  parent reply	other threads:[~2021-07-07 10:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-02  8:13 [PATCH] use 64bit timer for hpet zhaoyan.liao
2021-07-02 15:57 ` kernel test robot
2021-07-07 10:04 ` Thomas Gleixner [this message]
2021-07-08  3:11   ` Linux
2021-07-08 11:17     ` Thomas Gleixner
2021-07-12  4:52       ` Linux
2021-07-12  7:25         ` Thomas Gleixner
2021-07-13  1:43           ` Linux

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875yxmqw2s.ffs@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=dwmw@amazon.co.uk \
    --cc=guancheng.rjk@alibaba-inc.com \
    --cc=hpa@zytor.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=likunkun@bytedance.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=songmuchun@bytedance.com \
    --cc=zhaoyan.liao@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).