[PATCH] acpi/riscv: use a dedicated insertion point in riscv_acpi_register_ext_intc()

Maoyi Xie posted 1 patch 2 weeks ago
drivers/acpi/riscv/irq.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
[PATCH] acpi/riscv: use a dedicated insertion point in riscv_acpi_register_ext_intc()
Posted by Maoyi Xie 2 weeks ago
riscv_acpi_register_ext_intc() inserts into ext_intc_list, which
is sorted with the largest gsi_base first. It walks the list
until it finds the first entry with a smaller gsi_base, then
inserts the new entry before that entry. After the loop it also
caps the predecessor's nr_irqs if that predecessor was registered
as PENDING.

The current code uses the loop cursor for both jobs:

    list_for_each_entry(node, &ext_intc_list, list) {
            if (node->gsi_base < ext_intc_element->gsi_base)
                    break;
    }

    prev = list_prev_entry(node, list);
    if (!list_entry_is_head(prev, &ext_intc_list, list)) {
            ...
    }

    list_add_tail(&ext_intc_element->list, &node->list);

If the loop falls through (no entry has a smaller gsi_base),
node ends up past the end of the list. `&node->list` resolves
to `&ext_intc_list` via container_of() offset cancellation. So
list_prev_entry() lands on the last real entry, and
list_add_tail() inserts at the tail. The code works today.

It is fragile though. Any future change that reads another field
of node will hit memory before the ext_intc_list global.

Track the insertion point with a dedicated list_head pointer.
Initialise pos to `&ext_intc_list`. Set it to `&node->list` on
early break. Use pos->prev for the PENDING adjustment, and pos
for list_add_tail(). The cursor is no longer touched after the
loop. Behaviour is unchanged.

Same shape as the Koschel cleanups from 2022 (e.g. 99d8ae4ec8a
tracing, 2966a9918df clockevents, dc1acd5c946 dlm).

Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
 drivers/acpi/riscv/irq.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
index 9b88d0993e88..49582cf4f4e2 100644
--- a/drivers/acpi/riscv/irq.c
+++ b/drivers/acpi/riscv/irq.c
@@ -134,7 +134,8 @@ struct fwnode_handle *riscv_acpi_get_gsi_domain_id(u32 gsi)
 static int __init riscv_acpi_register_ext_intc(u32 gsi_base, u32 nr_irqs, u32 nr_idcs,
 					       u32 id, u32 type)
 {
-	struct riscv_ext_intc_list *ext_intc_element, *node, *prev;
+	struct riscv_ext_intc_list *ext_intc_element, *node;
+	struct list_head *pos = &ext_intc_list;
 
 	ext_intc_element = kzalloc_obj(*ext_intc_element);
 	if (!ext_intc_element)
@@ -153,18 +154,22 @@ static int __init riscv_acpi_register_ext_intc(u32 gsi_base, u32 nr_irqs, u32 nr
 	ext_intc_element->nr_idcs = nr_idcs;
 	ext_intc_element->id = id;
 	list_for_each_entry(node, &ext_intc_list, list) {
-		if (node->gsi_base < ext_intc_element->gsi_base)
+		if (node->gsi_base < ext_intc_element->gsi_base) {
+			pos = &node->list;
 			break;
+		}
 	}
 
 	/* Adjust the previous node's GSI range if that has pending registration */
-	prev = list_prev_entry(node, list);
-	if (!list_entry_is_head(prev, &ext_intc_list, list)) {
+	if (pos->prev != &ext_intc_list) {
+		struct riscv_ext_intc_list *prev =
+			list_entry(pos->prev, struct riscv_ext_intc_list, list);
+
 		if (prev->flag & RISCV_ACPI_INTC_FLAG_PENDING)
 			prev->nr_irqs = ext_intc_element->gsi_base - prev->gsi_base;
 	}
 
-	list_add_tail(&ext_intc_element->list, &node->list);
+	list_add_tail(&ext_intc_element->list, pos);
 	return 0;
 }
 
-- 
2.34.1
Re: [PATCH] acpi/riscv: use a dedicated insertion point in riscv_acpi_register_ext_intc()
Posted by Sunil V L 5 hours ago
On Mon, May 25, 2026 at 1:14 PM Maoyi Xie <maoyixie.tju@gmail.com> wrote:
>
> riscv_acpi_register_ext_intc() inserts into ext_intc_list, which
> is sorted with the largest gsi_base first. It walks the list
> until it finds the first entry with a smaller gsi_base, then
> inserts the new entry before that entry. After the loop it also
> caps the predecessor's nr_irqs if that predecessor was registered
> as PENDING.
>
> The current code uses the loop cursor for both jobs:
>
>     list_for_each_entry(node, &ext_intc_list, list) {
>             if (node->gsi_base < ext_intc_element->gsi_base)
>                     break;
>     }
>
>     prev = list_prev_entry(node, list);
>     if (!list_entry_is_head(prev, &ext_intc_list, list)) {
>             ...
>     }
>
>     list_add_tail(&ext_intc_element->list, &node->list);
>
> If the loop falls through (no entry has a smaller gsi_base),
> node ends up past the end of the list. `&node->list` resolves
> to `&ext_intc_list` via container_of() offset cancellation. So
> list_prev_entry() lands on the last real entry, and
> list_add_tail() inserts at the tail. The code works today.
>
> It is fragile though. Any future change that reads another field
> of node will hit memory before the ext_intc_list global.
>
> Track the insertion point with a dedicated list_head pointer.
> Initialise pos to `&ext_intc_list`. Set it to `&node->list` on
> early break. Use pos->prev for the PENDING adjustment, and pos
> for list_add_tail(). The cursor is no longer touched after the
> loop. Behaviour is unchanged.
>
> Same shape as the Koschel cleanups from 2022 (e.g. 99d8ae4ec8a
> tracing, 2966a9918df clockevents, dc1acd5c946 dlm).
>
> Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
> ---
>  drivers/acpi/riscv/irq.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/riscv/irq.c b/drivers/acpi/riscv/irq.c
> index 9b88d0993e88..49582cf4f4e2 100644
> --- a/drivers/acpi/riscv/irq.c
> +++ b/drivers/acpi/riscv/irq.c
> @@ -134,7 +134,8 @@ struct fwnode_handle *riscv_acpi_get_gsi_domain_id(u32 gsi)
>  static int __init riscv_acpi_register_ext_intc(u32 gsi_base, u32 nr_irqs, u32 nr_idcs,
>                                                u32 id, u32 type)
>  {
> -       struct riscv_ext_intc_list *ext_intc_element, *node, *prev;
> +       struct riscv_ext_intc_list *ext_intc_element, *node;
> +       struct list_head *pos = &ext_intc_list;
>
>         ext_intc_element = kzalloc_obj(*ext_intc_element);
>         if (!ext_intc_element)
> @@ -153,18 +154,22 @@ static int __init riscv_acpi_register_ext_intc(u32 gsi_base, u32 nr_irqs, u32 nr
>         ext_intc_element->nr_idcs = nr_idcs;
>         ext_intc_element->id = id;
>         list_for_each_entry(node, &ext_intc_list, list) {
> -               if (node->gsi_base < ext_intc_element->gsi_base)
> +               if (node->gsi_base < ext_intc_element->gsi_base) {
> +                       pos = &node->list;
>                         break;
> +               }
>         }
>
>         /* Adjust the previous node's GSI range if that has pending registration */
> -       prev = list_prev_entry(node, list);
> -       if (!list_entry_is_head(prev, &ext_intc_list, list)) {
> +       if (pos->prev != &ext_intc_list) {
> +               struct riscv_ext_intc_list *prev =
> +                       list_entry(pos->prev, struct riscv_ext_intc_list, list);
> +
>                 if (prev->flag & RISCV_ACPI_INTC_FLAG_PENDING)
>                         prev->nr_irqs = ext_intc_element->gsi_base - prev->gsi_base;
>         }
>
> -       list_add_tail(&ext_intc_element->list, &node->list);
> +       list_add_tail(&ext_intc_element->list, pos);
>         return 0;
>  }
>
LGTM.

Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>