[email protected] writes:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [Medium] irqchip/gic-v3-its: incomplete NUMA_NO_NODE resolution exposes
> allocations to task mempolicy
> --
>
> commit 6d762ab83972132164ae57c1422f1916decb2fe3
> Author: Aneesh Kumar K.V (Arm) <[email protected]>
>
> irqchip/gic-v3-its: Resolve the default NUMA node explicitly
>
> alloc_pages_node() resolves NUMA_NO_NODE to numa_mem_id() internally. In
> preparation for switching ITS allocations to the CoCo shared memory
> allocator, resolve the default node explicitly in its_alloc_pages().
>
> The CoCo shared memory allocator interprets NUMA_NO_NODE as a request to
> apply the current task's memory policy. Passing numa_mem_id() instead
> preserves the existing nearest-memory-node placement across the
> allocator switch. This patch does not change the current allocation
> behavior.
>
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c
>> b/drivers/irqchip/irq-gic-v3-its.c
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -236,7 +237,7 @@ static struct page *its_alloc_pages_node(int node, gfp_t
>> gfp,
>>
>> static struct page *its_alloc_pages(gfp_t gfp, unsigned int order)
>> {
>> - return its_alloc_pages_node(NUMA_NO_NODE, gfp, order);
>> + return its_alloc_pages_node(numa_mem_id(), gfp, order);
>> }
>
> [Severity: Medium]
> Will this leave other callers of its_alloc_pages_node() exposed to the
> task memory policy change?
>
> When a device probe or ITT allocation occurs with its->numa_node ==
> NUMA_NO_NODE (which is common when firmware lacks NUMA info), callers pass
> NUMA_NO_NODE directly to its_alloc_pages_node():
>
> drivers/irqchip/irq-gic-v3-its.c:its_alloc_pages_node() {
> ...
> page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
> ...
> }
>
> If node is NUMA_NO_NODE, it passes it directly to the underlying allocator.
> When the underlying allocator interprets NUMA_NO_NODE as a request to apply
> task mempolicy, these direct callers will be subjected to the current task's
> memory policy instead of bypassing it and using the local node.
>
> If a user binds the driver via sysfs under a restrictive task mempolicy,
> the allocations could land on suboptimal distant nodes or fail entirely.
>
> Should the NUMA_NO_NODE resolution logic be moved inside
> its_alloc_pages_node()
> to ensure all callers are protected?
Yes, updated the patch to perform the mapping in its_alloc_pages_node(),
so all callers are handled.
@@ -216,6 +217,9 @@ static struct page *its_alloc_pages_node(int node, gfp_t
gfp,
struct page *page;
int ret = 0;
+ if (node == NUMA_NO_NODE)
+ node = numa_mem_id();
+
page = alloc_pages_node(node, gfp | gfp_flags_quirk, order);
if (!page)
