Re: [PATCH] irq: simplify irq_im_handle_irq()

[Jiri Slaby <jirislaby@kernel.org>]

On 19. 07. 25, 23:18, Yury Norov wrote:
> From: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> 
> Hi Thomas,

This does not belong to a commit log ^^.

> The function calls bitmap_empty() for potentially every bit in
> work_ctx->pending, which makes a simple bitmap traverse O(N^2).
> Fix it by switching to the dedicated for_each_set_bit().

Looks good.

> While there, fix using atomic clear_bit() in a context where atomicity
> cannot be guaranteed.

What does this mean? __clear_bit() can corrupt the bitmap when there is 
an in-flight set_bit(), right?

> Signed-off-by: Yury Norov (NVIDIA) <yury.norov@gmail.com>
> ---
>   kernel/irq/irq_sim.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/irq/irq_sim.c b/kernel/irq/irq_sim.c
> index ae4c9cbd1b4b..e05904da7e3d 100644
> --- a/kernel/irq/irq_sim.c
> +++ b/kernel/irq/irq_sim.c
> @@ -128,15 +128,13 @@ 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;
> +	unsigned int offset;
>   	int irqnum;
>   
>   	work_ctx = container_of(work, struct irq_sim_work_ctx, work);
>   
> -	while (!bitmap_empty(work_ctx->pending, work_ctx->irq_count)) {
> -		offset = find_next_bit(work_ctx->pending,
> -				       work_ctx->irq_count, offset);
> -		clear_bit(offset, work_ctx->pending);
> +	for_each_set_bit(offset, work_ctx->pending, work_ctx->irq_count) {
> +		__clear_bit(offset, work_ctx->pending);
>   		irqnum = irq_find_mapping(work_ctx->domain, offset);
>   		handle_simple_irq(irq_to_desc(irqnum));
>   	}

-- 
js
suse labs