Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Also fetch node info for non-PCI devices #172

Merged
merged 1 commit into from Jan 28, 2021
Merged

Conversation

ryncsn
Copy link
Contributor

@ryncsn ryncsn commented Jan 28, 2021

Followring error is observed on some large servers:

[ 40.345505] irq 4: Affinity broken due to vector space exhaustion.

Irq 4 is on node 0 but irqbalance put it in node 1. Root cause is that irq 4 is not an from an PCI device so irqbalance failed to get the right numa node number.

This PR fixes this problem.

non-PCI devices could also be bind to a certain node. So if failed to
fetch the info from sysfs, try to get it from /proc/irq/<irq>/node.
@ryncsn
Copy link
Contributor Author

ryncsn commented Jan 28, 2021

Hmm, I just noticed there is a issue #169 after I making this PR. That issue is almost the same with the one I'm experiencing.

On my machine after applying this patch the issue is gone. Hopefully I'm getting it right, will this PR will fix the issue?

@nhorman
Copy link
Member

nhorman commented Jan 28, 2021

yeah, I think you did. We honestly shouldn't have to do this, as the kernel shouldn't issue that error just because the irq is affined to a non-local numa node, but regardless, its better to assign the irq to a local node anyway, so I'll take this.

@nhorman nhorman merged commit 0f28426 into Irqbalance:master Jan 28, 2021
Copy link
Contributor

@legoater legoater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ryncsn @nhorman ,

While working on a fix related to IPIs of hotplugged CPUs on PPC, I found out that if Linux maps an IRQ associated with an empty node (hence not exposed under /sys/devices/system/node/), this change will lead to a core dump in place_irq_in_node() because irq_numa_node(info) is NULL.

It would be good to protect the code with a test on place_irq_in_node() or with something like below :

diff --git a/classify.c b/classify.c
index c08144f432bb..9b4640cd4754 100644
--- a/classify.c
+++ b/classify.c
@@ -376,6 +376,11 @@ get_numa_node:
        else
                new->numa_node = get_numa_node(numa_node);
 
+       if (!new->numa_node) {
+               log(TO_CONSOLE, LOG_WARNING, "IRQ %d has an unknown node\n", irq);
+               new->numa_node = get_numa_node(NUMA_NO_NODE);
+       }
+
        cpus_setall(new->cpumask);
        if (devpath != NULL) {
                sprintf(path, "%s/local_cpus", devpath);

What would you prefer ?

Thanks,

C.

@nhorman
Copy link
Member

nhorman commented Aug 6, 2021

I think that would be fine, open a PR please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants