mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Andy Lutomirski" <luto@kernel.org>
To: "Per Bilse" <per.bilse@citrix.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Juergen Gross" <jgross@suse.com>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"moderated list:XEN HYPERVISOR INTERFACE" 
	<xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] Updates to Xen hypercall preemption
Date: Wed, 21 Jun 2023 09:27:31 -0700	[thread overview]
Message-ID: <7fbad052-4c4a-4a49-913d-ea836c180dc2@app.fastmail.com> (raw)
In-Reply-To: <20230621151442.2152425-1-per.bilse@citrix.com>

On Wed, Jun 21, 2023, at 8:14 AM, Per Bilse wrote:
> Some Xen hypercalls issued by dom0 guests may run for many 10s of
> seconds, potentially causing watchdog timeouts and other problems.
> It's rare for this to happen, but it does in extreme circumstances,
> for instance when shutting down VMs with very large memory allocations
> (> 0.5 - 1TB).  These hypercalls are preemptible, but the fixes in the
> kernel to ensure preemption have fallen into a state of disrepair, and
> are currently ineffective.  This patch brings things up to date by way of:
>
> 1) Update general feature selection from XEN_PV to XEN_DOM0.
> The issue is unique to dom0 Xen guests, but isn't unique to PV dom0s,
> and will occur in future PVH dom0s.  XEN_DOM0 depends on either PV or PVH,
> as well as the appropriate details for dom0.
>
> 2) Update specific feature selection from !PREEMPTION to !PREEMPT.
> The following table shows the relationship between different preemption
> features and their indicators/selectors (Y = "=Y", N = "is not set",
> . = absent):
>
>                             | np-s | np-d | vp-s | vp-d | fp-s | fp-d
>     CONFIG_PREEMPT_DYNAMIC      N      Y      N      Y      N      Y
>          CONFIG_PREEMPTION      .      Y      .      Y      Y      Y
>             CONFIG_PREEMPT      N      N      N      N      Y      Y
>   CONFIG_PREEMPT_VOLUNTARY      N      N      Y      Y      N      N
>        CONFIG_PREEMPT_NONE      Y      Y      N      N      N      N
>
> Unless PREEMPT is set, we need to enable the fixes.

This code is a horrible mess, with and without your patches.  I think that, if this were new, there's no way it would make it in to the kernel.

I propose one of two rather radical changes:

1. (preferred) Just delete all of it and make support for dom0 require either full or dynamic preempt, and make a dynamic preempt kernel booting as dom0 run as full preempt.

2. Forget about trying to preempt a hypercall in the sense of scheduling from an interrupt.  Instead teach the interrupt code to detect that it's in a preemptible hypercall and change RIP to a landing pad that does a cond_resched() and then resumes the hypercall.

I don't think the entry code should have a whole special preempt implementation just for this nasty special case.

--Andy

  reply	other threads:[~2023-06-21 16:27 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-21 15:14 Per Bilse
2023-06-21 16:27 ` Andy Lutomirski [this message]
2023-06-21 19:05   ` Per Bilse
2023-06-21 23:42     ` Andy Lutomirski
2023-06-21 16:40 ` Peter Zijlstra
2023-06-21 19:19   ` Per Bilse
2023-06-21 20:04     ` Peter Zijlstra
2023-06-22  5:22       ` Juergen Gross
2023-06-22  8:26         ` Peter Zijlstra
2023-06-22 10:33           ` Juergen Gross
2023-06-22 11:15             ` Peter Zijlstra
2023-06-22 12:17               ` Juergen Gross
2023-06-22 16:39             ` Andy Lutomirski
2023-06-22 17:20               ` Juergen Gross
2023-06-24 15:19                 ` Andy Lutomirski
2023-06-22 13:05           ` Andrew Cooper
2023-06-22 13:14             ` Peter Zijlstra
2023-06-22 13:35             ` Juergen Gross
2023-06-22 10:01       ` Per Bilse

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=7fbad052-4c4a-4a49-913d-ea836c180dc2@app.fastmail.com \
    --to=luto@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleksandr_tyshchenko@epam.com \
    --cc=per.bilse@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®