All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Andy Lutomirski <luto@kernel.org>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	Waiman Long <waiman.long@hpe.com>
Subject: Re: [PATCH] x86/vdso: Remove direct HPET access through the vDSO
Date: Fri, 8 Apr 2016 12:36:26 +0200	[thread overview]
Message-ID: <20160408103626.GD29522@pd.tnic> (raw)
In-Reply-To: <d2f90bba98db9905041cff294646d290d378f67a.1460074438.git.luto@kernel.org>

On Thu, Apr 07, 2016 at 05:16:59PM -0700, Andy Lutomirski wrote:
> Allowing user code to map the HPET is problematic.  HPET
> implementations are notoriously buggy, and there are probably many
> machines on which even MMIO reads from bogus HPET addresses are
> problematic.
> 
> We have a report that the Dell Precision M2800 with:
> 
> ACPI: HPET 0x00000000C8FE6238 000038 (v01 DELL   CBX3  01072009 AMI. 00000005)
> 
> is either so slow when accessing the HPET or actually hangs in some
> regard, causing soft lockups to be reported if users do unexpected
> things to the HPET.
> 
> The vclock HPET code has also always been a questionable speedup.
> Accessing an HPET is exceedingly slow (on the order of several
> microseconds), so the added overhead in requiring a syscall to read
> the HPET is a small fraction of the total code of accessing it.
> 
> To avoid future problems, let's just delete the code entirely.
> 
> In the long run, this could actually be a speedup.  Waiman Long as a
> patch to optimize the case where multiple CPUs contend for the HPET,
> but that won't help unless all the accesses are mediated by the
> kernel.
> 
> Cc: Waiman Long <waiman.long@hpe.com>
> Reported-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/entry/vdso/vclock_gettime.c  | 15 ---------------
>  arch/x86/entry/vdso/vdso-layout.lds.S |  5 ++---
>  arch/x86/entry/vdso/vma.c             | 11 -----------
>  arch/x86/include/asm/clocksource.h    |  9 ++++-----
>  arch/x86/kernel/hpet.c                |  1 -
>  arch/x86/kvm/trace.h                  |  3 +--
>  6 files changed, 7 insertions(+), 37 deletions(-)

I like diffstats which remove more lines than add :)

In any case, I'm not well versed in the vdso game but from my experience
so far, I'm starting to believe that the 'H' in HPET stands for
Horrific. So the less people get exposed to it, the better.

A question though: userspace does not rely on the hpet page being always
present and can do fallback. IOW, we're not breaking any existing
usages, right?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

  reply	other threads:[~2016-04-08 10:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-08  0:16 [PATCH] x86/vdso: Remove direct HPET access through the vDSO Andy Lutomirski
2016-04-08 10:36 ` Borislav Petkov [this message]
2016-04-08 15:59   ` Andy Lutomirski
2016-04-09 22:47 ` Thomas Gleixner
2016-04-13 11:31 ` [tip:x86/asm] " tip-bot for Andy Lutomirski

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=20160408103626.GD29522@pd.tnic \
    --to=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=waiman.long@hpe.com \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.