lkml.org 
[lkml]   [2015]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC v2 PATCH 07/14] irq: add a new generic IPI reservation code to irq core
On Tue, 13 Oct 2015, Qais Yousef wrote:
> +/**
> + * irq_reserve_ipi() - setup an IPI to destination cpumask
> + * @domain: IPI domain
> + * @dest: cpumask of cpus to receive the IPI
> + *
> + * Allocate a virq that can be used to send IPI to any CPU in dest mask.
> + *
> + * On success it'll return linux irq number and 0 on failure
> + */
> +unsigned int irq_reserve_ipi(struct irq_domain *domain,
> + const struct ipi_mask *dest)
> +{
> + struct irq_data *data;
> + int virq;
> + unsigned int nr_irqs;

Please order them so:

+ struct irq_data *data;
+ unsigned int nr_irqs;
+ int virq;

Much simpler to read.

> + if (domain == NULL)
> + domain = irq_default_domain; /* need a separate ipi_default_domain? */

No tail comments please.

We should neither use irq_default_domain nor have an
ipi_default_domain.

> +
> + if (domain == NULL) {
> + pr_warn("Must provide a valid IPI domain!\n");
> + return 0;
> + }
> +
> + if (!irq_domain_is_ipi(domain)) {
> + pr_warn("Not an IPI domain!\n");
> + return 0;
> + }
> +
> + /* always allocate a virq per cpu */
> + nr_irqs = bitmap_weight(dest->cpumask, dest->nbits);;

Double semicolon

> +
> + virq = irq_domain_alloc_descs(-1, nr_irqs, 0, NUMA_NO_NODE);
> + if (virq <= 0) {
> + pr_warn("Can't reserve IPI, failed to alloc descs\n");
> + return 0;
> + }
> +
> + /* we are reusing hierarchy alloc function, should we create another one? */
> + virq = __irq_domain_alloc_irqs(domain, virq, nr_irqs, NUMA_NO_NODE,
> + (void *) dest, true);
> + if (virq <= 0) {
> + pr_warn("Can't reserve IPI, failed to alloc irqs\n");
> + goto free_descs;
> + }
> +
> + data = irq_get_irq_data(virq);
> + bitmap_copy(data->ipi_mask.cpumask, dest->cpumask, dest->nbits);
> + data->ipi_mask.nbits = dest->nbits;

This does only initialize the first virq data. What about the others?

> + return virq;
> +
> +free_descs:
> + irq_free_descs(virq, nr_irqs);
> + return 0;
> +}
> +
> +/**
> + * irq_destroy_ipi() - unreserve an IPI that was previously allocated
> + * @irq: linux irq number to be destroyed
> + *
> + * Return an IPI allocated with irq_reserve_ipi() to the system.

That wants to explain that it actually destroys a number of virqs not
just the primary one.

Thanks,

tglx


\
 
 \ /
  Last update: 2015-10-13 16:01    [W:0.318 / U:1.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site