[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] xen/ioapic: Don't use local_irq_restore() to disable irqs


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 21 Feb 2023 14:40:39 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=aG4+Rvk4oBN/gBEzlbqVzRZTGMIwzdsIKympAT5ijNY=; b=T/7guqhvLcHj/j/XeVH0lLEcNfetGoPKd0ZtT3cDkPoFhMumD0EuHU0OkB1EV8fKpkfrCQh3M46McdJnlr96w4BzRQmwtVc1N1B84dFSAiZX5VT/5sPllxLfJjFMQAigmiJyskNbLUI+BsKn8qTR6FKPojYcvCBKQYUR3b70+uy8uEt655RWAmt+lV6td6p6Wspo5VHCCDp1Fcmis6wgebdmtvKt1bCtkf0sNBxpo0DcmhT462Lj6uOdVaxj9TVocaNRcb9bK4GfzIOndA1uBLdZRnNjYT32S2Nwa2YfGjNDWH+zAjILsuFt46klN7Q5z1M5WWt5Tv3Grns16HlE8Q==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hsl5nW/UExFWDDDqz9tLUtkBal9Dx2JIxR9RsSSVkQtIcRE3ob2e0YxPqPNwfsJrdt6jJKXEC/tn/3ZXdyRlkPud+B5Pd33GusprfUGTxvujw7a0u5SWEnITbkXsVbQOtzPQZ9uIxp4wUbBNIH5b5etJ4EPa6YENBF6dJJjyfFIT3+rxoGYR7jO2I9XRBtOs//DTlVHWqiyfipb4PCfapKgjh6Pj6uYAYtwDFxcL0jOByyyktJhxlg2hJf9jIUHwTQgM6wSRGZ+LBM5ZmS1sH8vWxaGhjji8qrZ4LzW9G+ACspqBnRmxSw1aB6/tTKlTUWKJJkpOEvecOtFAcFkpOQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 21 Feb 2023 13:40:46 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 20.02.2023 20:47, Andrew Cooper wrote:
> Despite its name, the irq{save,restore}() APIs are only intended to
> conditionally disable and re-enable interrupts.

Are they? Maybe nowadays they indeed are, but I couldn't spot any wording
to this effect in Linux'es Documentation/ (and I don't think we have
anywhere such could be put). Yet I'd expect such a statement to be backed
by something.

> IO-APIC's timer_irq_works() violates this intention.  As it is init code,
> switch to simple irq enable/disable().

If all callers of the function obviously did have interrupts off, I might
agree. But the last of them sits _after_ local_irq_restore(), and all of
this is called from underneath smp_prepare_cpus(), i.e. after IRQs were
already enabled. I'm willing to believe that the local_irq_restore()
there really comes too early, but then, ...

> No functional change.

... in order for this to be true, that'll need fixing at the same time (or
in a prereq patch).

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.