linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap
@ 2018-11-09 17:21 Bartosz Golaszewski
  2018-12-11 14:02 ` Bartosz Golaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: Bartosz Golaszewski @ 2018-11-09 17:21 UTC (permalink / raw)
  To: Thomas Gleixner, Uwe Kleine-König; +Cc: linux-kernel, Bartosz Golaszewski

Two threads can try to fire the irq_sim with different offsets and will
end up fighting for the irq_work asignment. Thomas Gleixner suggested a
solution based on a bitfield where we set a bit for every offset
associated with an interrupt that should be fired and then iterate over
all set bits in the interrupt handler.

This is a slightly modified solution using a bitmap so that we don't
impose a limit on the number of interrupts one can allocate with
irq_sim.

Suggested-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 include/linux/irq_sim.h |  2 +-
 kernel/irq/irq_sim.c    | 23 +++++++++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
index 630a57e55db6..4500d453a63e 100644
--- a/include/linux/irq_sim.h
+++ b/include/linux/irq_sim.h
@@ -16,7 +16,7 @@
 
 struct irq_sim_work_ctx {
 	struct irq_work		work;
-	int			irq;
+	unsigned long		*pending;
 };
 
 struct irq_sim_irq_ctx {
diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
index dd20d0d528d4..98a20e1594ce 100644
--- a/kernel/irq/irq_sim.c
+++ b/kernel/irq/irq_sim.c
@@ -34,9 +34,20 @@ static struct irq_chip irq_sim_irqchip = {
 static void irq_sim_handle_irq(struct irq_work *work)
 {
 	struct irq_sim_work_ctx *work_ctx;
+	unsigned int offset = 0;
+	struct irq_sim *sim;
+	int irqnum;
 
 	work_ctx = container_of(work, struct irq_sim_work_ctx, work);
-	handle_simple_irq(irq_to_desc(work_ctx->irq));
+	sim = container_of(work_ctx, struct irq_sim, work_ctx);
+
+	while (!bitmap_empty(work_ctx->pending, sim->irq_count)) {
+		offset = find_next_bit(work_ctx->pending,
+				       sim->irq_count, offset);
+		clear_bit(offset, work_ctx->pending);
+		irqnum = irq_sim_irqnum(sim, offset);
+		handle_simple_irq(irq_to_desc(irqnum));
+	}
 }
 
 /**
@@ -63,6 +74,13 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
 		return sim->irq_base;
 	}
 
+	sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
+	if (!sim->work_ctx.pending) {
+		kfree(sim->irqs);
+		irq_free_descs(sim->irq_base, num_irqs);
+		return -ENOMEM;
+	}
+
 	for (i = 0; i < num_irqs; i++) {
 		sim->irqs[i].irqnum = sim->irq_base + i;
 		sim->irqs[i].enabled = false;
@@ -89,6 +107,7 @@ EXPORT_SYMBOL_GPL(irq_sim_init);
 void irq_sim_fini(struct irq_sim *sim)
 {
 	irq_work_sync(&sim->work_ctx.work);
+	bitmap_free(sim->work_ctx.pending);
 	irq_free_descs(sim->irq_base, sim->irq_count);
 	kfree(sim->irqs);
 }
@@ -143,7 +162,7 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
 void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
 {
 	if (sim->irqs[offset].enabled) {
-		sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
+		set_bit(offset, sim->work_ctx.pending);
 		irq_work_queue(&sim->work_ctx.work);
 	}
 }
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap
  2018-11-09 17:21 [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap Bartosz Golaszewski
@ 2018-12-11 14:02 ` Bartosz Golaszewski
  2018-12-11 14:41   ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Bartosz Golaszewski @ 2018-12-11 14:02 UTC (permalink / raw)
  To: Thomas Gleixner, Uwe Kleine-König, Marc Zyngier
  Cc: Linux Kernel Mailing List

pt., 9 lis 2018 o 18:21 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> Two threads can try to fire the irq_sim with different offsets and will
> end up fighting for the irq_work asignment. Thomas Gleixner suggested a
> solution based on a bitfield where we set a bit for every offset
> associated with an interrupt that should be fired and then iterate over
> all set bits in the interrupt handler.
>
> This is a slightly modified solution using a bitmap so that we don't
> impose a limit on the number of interrupts one can allocate with
> irq_sim.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> ---
>  include/linux/irq_sim.h |  2 +-
>  kernel/irq/irq_sim.c    | 23 +++++++++++++++++++++--
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
> index 630a57e55db6..4500d453a63e 100644
> --- a/include/linux/irq_sim.h
> +++ b/include/linux/irq_sim.h
> @@ -16,7 +16,7 @@
>
>  struct irq_sim_work_ctx {
>         struct irq_work         work;
> -       int                     irq;
> +       unsigned long           *pending;
>  };
>
>  struct irq_sim_irq_ctx {
> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> index dd20d0d528d4..98a20e1594ce 100644
> --- a/kernel/irq/irq_sim.c
> +++ b/kernel/irq/irq_sim.c
> @@ -34,9 +34,20 @@ static struct irq_chip irq_sim_irqchip = {
>  static void irq_sim_handle_irq(struct irq_work *work)
>  {
>         struct irq_sim_work_ctx *work_ctx;
> +       unsigned int offset = 0;
> +       struct irq_sim *sim;
> +       int irqnum;
>
>         work_ctx = container_of(work, struct irq_sim_work_ctx, work);
> -       handle_simple_irq(irq_to_desc(work_ctx->irq));
> +       sim = container_of(work_ctx, struct irq_sim, work_ctx);
> +
> +       while (!bitmap_empty(work_ctx->pending, sim->irq_count)) {
> +               offset = find_next_bit(work_ctx->pending,
> +                                      sim->irq_count, offset);
> +               clear_bit(offset, work_ctx->pending);
> +               irqnum = irq_sim_irqnum(sim, offset);
> +               handle_simple_irq(irq_to_desc(irqnum));
> +       }
>  }
>
>  /**
> @@ -63,6 +74,13 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
>                 return sim->irq_base;
>         }
>
> +       sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> +       if (!sim->work_ctx.pending) {
> +               kfree(sim->irqs);
> +               irq_free_descs(sim->irq_base, num_irqs);
> +               return -ENOMEM;
> +       }
> +
>         for (i = 0; i < num_irqs; i++) {
>                 sim->irqs[i].irqnum = sim->irq_base + i;
>                 sim->irqs[i].enabled = false;
> @@ -89,6 +107,7 @@ EXPORT_SYMBOL_GPL(irq_sim_init);
>  void irq_sim_fini(struct irq_sim *sim)
>  {
>         irq_work_sync(&sim->work_ctx.work);
> +       bitmap_free(sim->work_ctx.pending);
>         irq_free_descs(sim->irq_base, sim->irq_count);
>         kfree(sim->irqs);
>  }
> @@ -143,7 +162,7 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
>  void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
>  {
>         if (sim->irqs[offset].enabled) {
> -               sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
> +               set_bit(offset, sim->work_ctx.pending);
>                 irq_work_queue(&sim->work_ctx.work);
>         }
>  }
> --
> 2.19.1
>

Ping and Cc'ing Marc Zyngier. Any chance of getting this in for 4.21?

Bart

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap
  2018-12-11 14:02 ` Bartosz Golaszewski
@ 2018-12-11 14:41   ` Marc Zyngier
  2018-12-11 15:39     ` Bartosz Golaszewski
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2018-12-11 14:41 UTC (permalink / raw)
  To: Bartosz Golaszewski, Thomas Gleixner, Uwe Kleine-König
  Cc: Linux Kernel Mailing List

On 11/12/2018 14:02, Bartosz Golaszewski wrote:
> pt., 9 lis 2018 o 18:21 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>>
>> Two threads can try to fire the irq_sim with different offsets and will
>> end up fighting for the irq_work asignment. Thomas Gleixner suggested a
>> solution based on a bitfield where we set a bit for every offset
>> associated with an interrupt that should be fired and then iterate over
>> all set bits in the interrupt handler.
>>
>> This is a slightly modified solution using a bitmap so that we don't
>> impose a limit on the number of interrupts one can allocate with
>> irq_sim.
>>
>> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
>> ---
>>  include/linux/irq_sim.h |  2 +-
>>  kernel/irq/irq_sim.c    | 23 +++++++++++++++++++++--
>>  2 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
>> index 630a57e55db6..4500d453a63e 100644
>> --- a/include/linux/irq_sim.h
>> +++ b/include/linux/irq_sim.h
>> @@ -16,7 +16,7 @@
>>
>>  struct irq_sim_work_ctx {
>>         struct irq_work         work;
>> -       int                     irq;
>> +       unsigned long           *pending;
>>  };
>>
>>  struct irq_sim_irq_ctx {
>> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
>> index dd20d0d528d4..98a20e1594ce 100644
>> --- a/kernel/irq/irq_sim.c
>> +++ b/kernel/irq/irq_sim.c
>> @@ -34,9 +34,20 @@ static struct irq_chip irq_sim_irqchip = {
>>  static void irq_sim_handle_irq(struct irq_work *work)
>>  {
>>         struct irq_sim_work_ctx *work_ctx;
>> +       unsigned int offset = 0;
>> +       struct irq_sim *sim;
>> +       int irqnum;
>>
>>         work_ctx = container_of(work, struct irq_sim_work_ctx, work);
>> -       handle_simple_irq(irq_to_desc(work_ctx->irq));
>> +       sim = container_of(work_ctx, struct irq_sim, work_ctx);
>> +
>> +       while (!bitmap_empty(work_ctx->pending, sim->irq_count)) {
>> +               offset = find_next_bit(work_ctx->pending,
>> +                                      sim->irq_count, offset);
>> +               clear_bit(offset, work_ctx->pending);
>> +               irqnum = irq_sim_irqnum(sim, offset);
>> +               handle_simple_irq(irq_to_desc(irqnum));
>> +       }
>>  }
>>
>>  /**
>> @@ -63,6 +74,13 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
>>                 return sim->irq_base;
>>         }
>>
>> +       sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
>> +       if (!sim->work_ctx.pending) {
>> +               kfree(sim->irqs);
>> +               irq_free_descs(sim->irq_base, num_irqs);
>> +               return -ENOMEM;
>> +       }
>> +
>>         for (i = 0; i < num_irqs; i++) {
>>                 sim->irqs[i].irqnum = sim->irq_base + i;
>>                 sim->irqs[i].enabled = false;
>> @@ -89,6 +107,7 @@ EXPORT_SYMBOL_GPL(irq_sim_init);
>>  void irq_sim_fini(struct irq_sim *sim)
>>  {
>>         irq_work_sync(&sim->work_ctx.work);
>> +       bitmap_free(sim->work_ctx.pending);
>>         irq_free_descs(sim->irq_base, sim->irq_count);
>>         kfree(sim->irqs);
>>  }
>> @@ -143,7 +162,7 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
>>  void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
>>  {
>>         if (sim->irqs[offset].enabled) {
>> -               sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
>> +               set_bit(offset, sim->work_ctx.pending);
>>                 irq_work_queue(&sim->work_ctx.work);
>>         }
>>  }
>> --
>> 2.19.1
>>
> 
> Ping and Cc'ing Marc Zyngier. Any chance of getting this in for 4.21?

Bizarrely, I can't find any trace of this patch in my Inbox. I've now
cherry-picked from https://lore.kernel.org/patchwork/patch/1009297/ .

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap
  2018-12-11 14:41   ` Marc Zyngier
@ 2018-12-11 15:39     ` Bartosz Golaszewski
  0 siblings, 0 replies; 4+ messages in thread
From: Bartosz Golaszewski @ 2018-12-11 15:39 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Thomas Gleixner, Uwe Kleine-König, Linux Kernel Mailing List

wt., 11 gru 2018 o 15:42 Marc Zyngier <marc.zyngier@arm.com> napisał(a):
>
> On 11/12/2018 14:02, Bartosz Golaszewski wrote:
> > pt., 9 lis 2018 o 18:21 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
> >>
> >> Two threads can try to fire the irq_sim with different offsets and will
> >> end up fighting for the irq_work asignment. Thomas Gleixner suggested a
> >> solution based on a bitfield where we set a bit for every offset
> >> associated with an interrupt that should be fired and then iterate over
> >> all set bits in the interrupt handler.
> >>
> >> This is a slightly modified solution using a bitmap so that we don't
> >> impose a limit on the number of interrupts one can allocate with
> >> irq_sim.
> >>
> >> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> >> Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> >> ---
> >>  include/linux/irq_sim.h |  2 +-
> >>  kernel/irq/irq_sim.c    | 23 +++++++++++++++++++++--
> >>  2 files changed, 22 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/linux/irq_sim.h b/include/linux/irq_sim.h
> >> index 630a57e55db6..4500d453a63e 100644
> >> --- a/include/linux/irq_sim.h
> >> +++ b/include/linux/irq_sim.h
> >> @@ -16,7 +16,7 @@
> >>
> >>  struct irq_sim_work_ctx {
> >>         struct irq_work         work;
> >> -       int                     irq;
> >> +       unsigned long           *pending;
> >>  };
> >>
> >>  struct irq_sim_irq_ctx {
> >> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> >> index dd20d0d528d4..98a20e1594ce 100644
> >> --- a/kernel/irq/irq_sim.c
> >> +++ b/kernel/irq/irq_sim.c
> >> @@ -34,9 +34,20 @@ static struct irq_chip irq_sim_irqchip = {
> >>  static void irq_sim_handle_irq(struct irq_work *work)
> >>  {
> >>         struct irq_sim_work_ctx *work_ctx;
> >> +       unsigned int offset = 0;
> >> +       struct irq_sim *sim;
> >> +       int irqnum;
> >>
> >>         work_ctx = container_of(work, struct irq_sim_work_ctx, work);
> >> -       handle_simple_irq(irq_to_desc(work_ctx->irq));
> >> +       sim = container_of(work_ctx, struct irq_sim, work_ctx);
> >> +
> >> +       while (!bitmap_empty(work_ctx->pending, sim->irq_count)) {
> >> +               offset = find_next_bit(work_ctx->pending,
> >> +                                      sim->irq_count, offset);
> >> +               clear_bit(offset, work_ctx->pending);
> >> +               irqnum = irq_sim_irqnum(sim, offset);
> >> +               handle_simple_irq(irq_to_desc(irqnum));
> >> +       }
> >>  }
> >>
> >>  /**
> >> @@ -63,6 +74,13 @@ int irq_sim_init(struct irq_sim *sim, unsigned int num_irqs)
> >>                 return sim->irq_base;
> >>         }
> >>
> >> +       sim->work_ctx.pending = bitmap_zalloc(num_irqs, GFP_KERNEL);
> >> +       if (!sim->work_ctx.pending) {
> >> +               kfree(sim->irqs);
> >> +               irq_free_descs(sim->irq_base, num_irqs);
> >> +               return -ENOMEM;
> >> +       }
> >> +
> >>         for (i = 0; i < num_irqs; i++) {
> >>                 sim->irqs[i].irqnum = sim->irq_base + i;
> >>                 sim->irqs[i].enabled = false;
> >> @@ -89,6 +107,7 @@ EXPORT_SYMBOL_GPL(irq_sim_init);
> >>  void irq_sim_fini(struct irq_sim *sim)
> >>  {
> >>         irq_work_sync(&sim->work_ctx.work);
> >> +       bitmap_free(sim->work_ctx.pending);
> >>         irq_free_descs(sim->irq_base, sim->irq_count);
> >>         kfree(sim->irqs);
> >>  }
> >> @@ -143,7 +162,7 @@ EXPORT_SYMBOL_GPL(devm_irq_sim_init);
> >>  void irq_sim_fire(struct irq_sim *sim, unsigned int offset)
> >>  {
> >>         if (sim->irqs[offset].enabled) {
> >> -               sim->work_ctx.irq = irq_sim_irqnum(sim, offset);
> >> +               set_bit(offset, sim->work_ctx.pending);
> >>                 irq_work_queue(&sim->work_ctx.work);
> >>         }
> >>  }
> >> --
> >> 2.19.1
> >>
> >
> > Ping and Cc'ing Marc Zyngier. Any chance of getting this in for 4.21?
>
> Bizarrely, I can't find any trace of this patch in my Inbox. I've now
> cherry-picked from https://lore.kernel.org/patchwork/patch/1009297/ .
>

Your e-mail doesn't pop up with get_maintainer on this patch. That's
why you didn't get it initially.

Bart

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-12-11 15:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-09 17:21 [PATCH] irq/irq_sim: store multiple interrupt offsets in a bitmap Bartosz Golaszewski
2018-12-11 14:02 ` Bartosz Golaszewski
2018-12-11 14:41   ` Marc Zyngier
2018-12-11 15:39     ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).