On Wed, Jun 16 2021 at 08:40, Ming Lei wrote:
> On Tue, Jun 15, 2021 at 02:57:07PM -0500, Bjorn Helgaas wrote:
> +static inline void irq_affinity_calc_sets_legacy(struct irq_affinity *affd)
This function name sucks because the function is really a wrapper around
irq_affinity_calc_sets(). What's so legacy about this? The fact that
it's called from the legacy PCI single interrupt code path?
> @@ -405,6 +405,30 @@ static void default_calc_sets(struct irq_affinity *affd, unsigned int affvecs)
> affd->set_size[0] = affvecs;
> }
>
> +static void irq_affinity_calc_sets(unsigned int affvecs,
> + struct irq_affinity *affd)
Please align the arguments when you need a line break.
> +{
> + /*
> + * Simple invocations do not provide a calc_sets() callback. Install
> + * the generic one.
> + */
> + if (!affd->calc_sets)
> + affd->calc_sets = default_calc_sets;
> +
> + /* Recalculate the sets */
> + affd->calc_sets(affd, affvecs);
> +
> + WARN_ON_ONCE(affd->nr_sets > IRQ_AFFINITY_MAX_SETS);
Hrm. That function really should return an error code to tell the caller
that something went wrong.
> +}
> +
> +/* Provide a chance to call ->calc_sets for legacy */
What does this comment tell? Close to zero.
> +void irq_affinity_calc_sets_legacy(struct irq_affinity *affd)
> +{
> + if (!affd)
> + return;
> + irq_affinity_calc_sets(0, affd);
> +}
What's wrong with just exposing irq_affinity_calc_sets() have that
NULL pointer check in the function and add proper function documentation
which explains what this is about?
Thanks,
tglx
