Messages in this thread | | | Date | Mon, 29 Oct 2018 22:29:05 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH] Choose CPU based on allocated IRQs |
| |
Long,
On Tue, 23 Oct 2018, Long Li wrote:
thanks for this patch.
A trivial formal thing ahead. The subject line
[PATCH] Choose CPU based on allocated IRQs
is lacking a proper subsystem prefix. In most cases you can figure the prefix out by running 'git log path/to/file' which in this case will show you that most commits touching this file use the prefix 'genirq/matrix:'.
So the proper subject would be:
[PATCH] genirq/matrix: Choose CPU based on allocated IRQs
Subsystem prefixes are important to see where a patch belongs to right from the subject. Without that it could belong to any random part of the kernel and needs further inspection of the patch itself. This applies to both email and to git shortlog listings.
> From: Long Li <longli@microsoft.com> > > In irq_matrix, "available" is set when IRQs are allocated earlier in the IRQ > assigning process. > > Later, when IRQs are activated those values are not good indicators of what > CPU to choose to assign to this IRQ.
Can you please explain why you think that available is the wrong indicator and which problem you are trying to solve?
The WHY is really the most important part of a changelog.
> Change to choose CPU for an IRQ based on how many IRQs are already allocated > on this CPU.
Looking deeper. The initial values are:
available = alloc_size - (managed + systembits) allocated = 0
There are two distinct functionalities which modify 'available' and 'allocated' (omitting the reverse operations for simplicity):
1) managed interrupts
reserve_managed() managed++; available--;
alloc_managed() allocated++;
2) regular interrupts
alloc() allocated++; available--;
So 'available' can be lower than 'allocated' depending on the number of reserved managed interrupts, which have not yet been activated.
So for all regular interrupts we really want to look at the number of 'available' vectors because the reserved managed ones are already accounted there and they need to be taken into account.
For the spreading of managed interrupts in alloc_managed() that's indeed a different story and 'allocated' is more correct. But even that is not completely accurate and can lead to the wrong result. The accurate solution would be to account the managed _and_ allocated vectors separately and do the spreading for managed interrupts based on that.
Thanks,
tglx
| |