irbalance-1.0.3 brings some very important changes and is of significantly greater utility than the preceding releases. In particular, it is now able to handle MSI/MSI-X interrupts correctly although this enhancement does require a kernel patch to expose the necessary information via sysfs (it will warn the user otherwise). This patch will be landing in kernel 3.3. I have amended the ebuild so that user is clearly informed of these developments through ewarn messages. I have also changed "numa" to be a pkginternal default. In my view, NUMA awareness is simply far too important to be overlooked. Moreover, having it off by default deviates from upstream. Bear in mind that NUMA systems are fairly common now, even more so with the advent of Intel's Nehalem architecture. It is vitally important for irqbalance to yield good results on such platforms and the numactl dependency is miniscule. Finally, I'll attach the ebuild diff along with the copy of Neil Horman's patch. It has long since been accepted by Greg Kroah-Hartman in its current form and accepted to the linux-2.6 git tree but it didn't make it time for the current 3.2 release. I have been using both the patch and new builds of irqbalance in my production environment for a while (12 core Xeons with multiple 8-queue NICs under heavy load) and the results have been good - equivalent to the performance I could only achieve before by manually assessing and tuning the affinity maps. I wish to see the patch committed to the portage tree (check the ewarn messages for the exact rationale). I have confirmed that the patch applies to 3.2 and 3.1 cleanly. In all probability, it applies to older versions just fine too but I haven't checked yet.
Created attachment 302931 [details, diff] diff from irqbalance-1.0.2-r1.ebuild
Created attachment 302933 [details, diff] pci-sysfs-add-per-device-msi-irq-listing.patch
Should the init.d script maybe refuse to run if the msi inodes are not detected?
Wow, fast reply there, Robin! One side of me thinks this is a good idea, especially if the output is not logged by default, or otherwise explicitly brought to the user's attention. If one runs irqbalance -o -d then the complaint will be quite visible but it's reasonable to assume that few people will actually do that. On the other hand, I'm wary of init scripts that try to be too clever because, sometimes, they can end up being unduly disruptive or being outright broken when things change. One example that springs to mind is conntrackd, which I recently started using. After first installing in Gentoo, it refused to even start; it tells me that TCP window tracking has to be disabled which, afaict, isn't true (certiainly, conntrackd has formal support for window tracking with >= linux-2..6.36). Because the ewarn information is clear, and because it seems implausible for irqbalance to be any more 'wrong' than prior releases, it doesn't strike me as essential. Then again, if it can be implemented reliably, it could prevent users from accidentally making things worse which is a worthy outcome. It seems that irqbalance doesn't log anything at all when started as a persistent instance from the init script so, if the user ignores the warnings from the ebuild, they may never know. Having written the above, my mind is now swayed towards thinking it'd be a good idea, as long as the logic in the init script is sound.
Here's a thought: I wonder if Mike (Pagano) might be amenable to adding the patch in genpatches? That would propagate to various sys-kernel/* ebuilds, not least, gentoo-sources.
How about this:- if [[ $(find /sys/devices -type d -name "msi_irqs" -printf . 2>/dev/null | wc -c) -eq 0 ]]; then # die here fi
Or ... if [[ -z "$(find /sys/devices -type d -name msi_irqs 2>/dev/null)" ]]; then # die here fi
Created attachment 302935 [details, diff] diff from irqbalance.init Sanity check added, as per Robin Johnson's suggestion. The error message is exactly the same as the warning that irqbalance emits in its debug mode.
I just realised that the new irq handling and balancing code has actually been there since v1.0 (somehow, I overlooked that I was running 0.56 before). I'll make the minor change required in the ebuild blurb and update the attachment.
Created attachment 303021 [details, diff] diff from irqbalance-1.0.2-r1.ebuild
(In reply to comment #8) > Created attachment 302935 [details, diff] [details, diff] > diff from irqbalance.init > > Sanity check added, as per Robin Johnson's suggestion. The error message is > exactly the same as the warning that irqbalance emits in its debug mode. [[ is a bashism that breaks for those of use who use /bin/dash to parse init scripts, please consider using test or [ instead.
Created attachment 304429 [details, diff] diff from irqbalance.init * Remove bashism as per request by Paolo Pedroni * Add "use iptables" because iptables policy should be instated before conntrackd is loaded
Created attachment 304433 [details, diff] diff from irqbalance.init Drop "use iptables" which has absolutely nothing to do with this bug and should be the subject of an entirely different bug I haven't even filed yet. Sorry about that.
So we likely still need to check in the init script for MSI bits and error out when its not found correct? Additionally we should say what the CONFIG_ value that you need to enable in your kernel is. Kerin, do you know off the top of your head?
Looks like we also want to ensure that CONFIG_PCI_MSI is enabled in the kernel config and warn if its not.
(In reply to comment #15) > Looks like we also want to ensure that CONFIG_PCI_MSI is enabled in the > kernel config and warn if its not. That seems like a good idea, provided that the wording is clear. Perhaps something to the effect of: "Modern PCs with a >=PCI 2.2 or PCI Express bus have support for message-signalled interrupts (MSI/MSI-X). If this applies to you then you should enable CONFIG_PCI_MSI for improved hardware performance and to allow irqbalance to better spread softirq load." The init script logic should be OK because it won't impede users whose machines/kernels don't support MSI. On the other hand, having a warning for CONFIG_PCI_MSI would provide guidance to those who can use MSI but have it disabled in the kernel for some reason. Although the patch is in >=3.3 and cannot be considered new at this point, it's conceivable that the users that really benefit from using irqbalance (i.e. people looking to aggressively scale network performance) might have a preference for older longterm kernels such as 3.2 or 3.0. For that reason, I think it would still be useful to implement these checks.
This should be all in CVS and going stable with 1.0.5
