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

Re: [PATCH for-4.22 v3 2/2] x86/io_apic: Use next_entry() in loops


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Jason Andryuk <jason.andryuk@xxxxxxx>
  • Date: Thu, 16 Oct 2025 20:28:39 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=suse.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0)
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=aL4lpS9pti12mMfEZKIJ14I3hWL8n8ML/yS5P8FT0Ig=; b=EK/6nRFgCzKy/5KegKEsUXJ6h6wSmWlCd7odRhIdF0E/4P3yk5eM2P3ZDc7b+JuUMVhOA4VnC50MNUvpHxqiqLC1V8cjQjy10w8c2wS95MEe16ReKbbudWsEi/SJzV6/wrEuBiVgRve5vvcWylZDTR2+mpOGkWsM/7eWE0MvdVMOWsUh1qZqbNfVXi50fmjkoUrjefDVpAG8ubcPse3wvQti0MOSbh43Ce0d8Wmd6JSef4TxHMGlewwbA2ydOTAnmv2bxLgS6dsBcPPMSYJ2Jrc4AC75+aWkovA4haeX5WjggqcD72pNJ3TRbn18kPA3ZYIgdXDXM2ZuXBGCMd0FSw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Z2tGFVGz5k4Zs5QCUGXtkKFNXZUBY4C/T+1LWz4ae7YQY98wZs2KPuSVcj2207mqa3MqJI4i6MPpOp6j3MgzqsuO5kDYE+2X5KOwxCGCKGuucnrMi4lEBWqq9crMOys8A/UK6f4KJ5IiV1ZUMTKA9qVKkGR815N5KexqNt8+2/0+NqRAMHmsqgliyhxBH6taI2R2TdIWJpPIMQkfZFKzbxCw8UZCzM5GpLCZhPKsqKHadr25946FjMtcu33pKqzYQUvKOcjFsKjGJ0PSX3QKqjGgb0S0Gdata91pF7+YYiTXG/IOpfC6ILME/wbY2iSfGQO5PLKx2wffgXPCzWAsng==
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 17 Oct 2025 00:29:11 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2025-10-16 02:53, Jan Beulich wrote:
On 15.10.2025 23:04, Jason Andryuk wrote:
io_apic.c has a lot of ad-hoc for(;;) and while(1) loops for iterating
over irq_pin_list entries.  Replace them with a standardized
for loop using next_entry() to advance entry.

Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
---
  xen/arch/x86/io_apic.c | 49 ++++++++++++------------------------------
  1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/xen/arch/x86/io_apic.c b/xen/arch/x86/io_apic.c
index c35d611ecf..73b48a9cb8 100644
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -191,6 +191,14 @@ static void remove_pin_from_irq(unsigned int irq, int 
apic, int pin)
      irq_2_pin_free_entry = entry - irq_2_pin;
  }
+static struct irq_pin_list *next_entry(const struct irq_pin_list *entry)
+{
+    if ( !entry->next )
+        return NULL;
+
+    return irq_2_pin + entry->next;
+}

Preferably with the function put in its final place right in patch 1:
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

@@ -482,7 +487,7 @@ static void modify_IO_APIC_irq(unsigned int irq, unsigned 
int enable,
  {
      struct irq_pin_list *entry = irq_2_pin + irq;
- for (;;) {
+    for (; entry; entry = next_entry(entry)) {
          unsigned int pin = entry->pin;
          struct IO_APIC_route_entry rte;
@@ -492,9 +497,6 @@ static void modify_IO_APIC_irq(unsigned int irq, unsigned int enable,
          rte.raw &= ~(uint64_t)disable;
          rte.raw |= enable;
          __ioapic_write_entry(entry->apic, pin, false, rte);
-        if (!entry->next)
-            break;
-        entry = irq_2_pin + entry->next;
      }
  }

I notice that within here there's also a "break" upon ->pin being -1.
Seeing that io_apic_level_ack_pending() has continue there, I think we
will want to be consistent. Which way isn't quite clear to me (yet).

Right. I don't know. It seems like ->pin == -1 indicates an unused entry, so stopping in the case makes sense. I've wondered if io_apic_level_ack_pending() continues just in case an entry->next points to another pin to ack. i.e. it's not the expected case, but it also might help if entry->next points to something valid.

Regards,
Jason



 


Rackspace

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