Re: [PATCH v10 2/2] irqchip/loongarch-avec: Add AVEC irqchip support

[Thomas Gleixner <tglx@linutronix.de>]

Huacai!

On Thu, Aug 22 2024 at 19:53, Huacai Chen wrote:
> On Wed, Aug 21, 2024 at 10:31 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>>
>> On Wed, Aug 21 2024 at 21:14, Huacai Chen wrote:
>> > On Wed, Aug 21, 2024 at 12:29 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> This patch is doing too many things at once and is absolutely not
>> >> reviewable.
>> >>
>> >> Please split it up into the obvious bits and pieces:
>> > Splitting may cause another problem: some patches will get upstream
>> > via the arch tree and others via the irq tree. These dependencies may
>> > cause build errors in a certain tree. But anyway, we will try our best
>> > to do this.
>>
>> That's not a problem at all. The trivial way to solve this is to apply
>> the architecture changes to the loongarch tree in a separate branch
>> which is based of some -rcX tag and only contains those dependencies.
>> That branch is then merged into the main loongarch branch and I can pull
>> it in to my tree for adding the irqchip changes. No conflicts, no merge
>> dependencies, nothing.
> Emm, another way is apply all patches to the irq tree with my Acked-by.

Correct, but that has the potential of creating conflicts when the
loongarch tree grows changes in the same areas.

>> > These macros are not in hot-path, and we have already tried our best
>> > to avoid using #ifdefs for cpu_has_xxx, so I suggest not introduce a
>> > new Kconfig option. Moreover, the new option should always be selected
>> > due to the deep coupling among loongson's irqchips, which makes the
>> > #ifdefs useless.
>>
>> They are removed in step 8 again. It's for having a sanely split up and
>> structured patch series instead of one big lump.
> I see, but I'm trying another splitting way to avoid
> adding-and-then-removing, of course it should also make reviews easy.

That's the whole point of the exercise.

>> The complexity of the x86 allocation/activate/set_affinity mechanisms
>> is there for a reason and not just because we did not have anything
>> better to do. :)
>
> Frankly, I haven't absorbed everything here, but I think I can try to
> answer my question "can irq_create_affinity_masks() still work".
>
> irq_create_affinity_masks() can still mark interrupts "managed" if
> avecintc driver doesn't support "managed", but it cannot guarantee
> that set_affinity can always succeed. If the destination cpu has a
> free vector, then set_affinity succeeds, otherwise it will fail. But
> if avecintc driver supports "managed", set_affinity can always
> succeed, because the destination cpu has already reserved a vector for
> this. Am I right?

It can work by some definition of "works", but in case of vector
exhaustion it will fail which is contrary to the purpose of managed
interrupts.

The matrix allocator already provides all the infrastructure and the
x86 reference implementation does the right thing. So why do you want to
shortcut that and make loongarch a special snowflake?

Thanks,

        tglx