[bug report] irq_work: Allow irq_work_sync() to sleep if irq_work() no IRQ support.

[bug report] irq_work: Allow irq_work_sync() to sleep if irq_work() no IRQ support.

[Dan Carpenter]

Hello Sebastian Andrzej Siewior,

The patch 810979682ccc: "irq_work: Allow irq_work_sync() to sleep if
irq_work() no IRQ support." from Oct 6, 2021, leads to the following
Smatch static checker warning:

	kernel/events/core.c:4955 _free_event()
	warn: sleeping in atomic context

There are two potential call trees where preemption is disabled.

perf_remove_from_context() <- disables preempt
__perf_event_exit_context() <- disables preempt
-> __perf_remove_from_context()
   -> perf_group_detach()
      -> perf_put_aux_event()
         -> put_event()
            -> _free_event()

_free_event() calls irq_work_sync() which can now sleep.

regards,
dan carpenter

Re: [bug report] irq_work: Allow irq_work_sync() to sleep if irq_work() no IRQ support.

[Sebastian Andrzej Siewior]

+peterz, tglx

On 2022-01-19 16:37:29 [+0300], Dan Carpenter wrote:
> Hello Sebastian Andrzej Siewior,
> 
> The patch 810979682ccc: "irq_work: Allow irq_work_sync() to sleep if
> irq_work() no IRQ support." from Oct 6, 2021, leads to the following
> Smatch static checker warning:
> 
> 	kernel/events/core.c:4955 _free_event()
> 	warn: sleeping in atomic context
> 
> There are two potential call trees where preemption is disabled.
> 
> perf_remove_from_context() <- disables preempt
 raw_spin_lock_irq(&ctx->lock)

> __perf_event_exit_context() <- disables preempt
don't see additional one.

> -> __perf_remove_from_context()
>    -> perf_group_detach()
>       -> perf_put_aux_event()
>          -> put_event()
>             -> _free_event()
> 
> _free_event() calls irq_work_sync() which can now sleep.

That is true. There is also 
|	if (event->rb) 
|		mutex_lock(&event->mmap_mutex);

Can we somehow delay the whole thing? There are also kfree() and
spinlock_t usage which are not very RT compatible with disabled
preemption.

> regards,
> dan carpenter

Sebastian

Re: [bug report] irq_work: Allow irq_work_sync() to sleep if irq_work() no IRQ support.

[Dan Carpenter]

On Wed, Jan 26, 2022 at 01:11:13PM +0100, Sebastian Andrzej Siewior wrote:
> 
> > __perf_event_exit_context() <- disables preempt
> don't see additional one.
> 
> > -> __perf_remove_from_context()

In linux-next perf_remove_from_context() calls raw_spin_lock() which
disabled preemption.

kernel/events/core.c
 13343        static void __perf_event_exit_context(void *__info)
 13344        {
 13345                struct perf_event_context *ctx = __info;
 13346                struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
 13347                struct perf_event *event;
 13348
 13349                raw_spin_lock(&ctx->lock);
                      ^^^^^^^^^^^^^^^^^^^^^^^^^
 13350                ctx_sched_out(ctx, cpuctx, EVENT_TIME);
 13351                list_for_each_entry(event, &ctx->event_list, event_entry)
 13352                        __perf_remove_from_context(event, cpuctx, ctx, (void *)DETACH_GROUP);
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 13353                raw_spin_unlock(&ctx->lock);
 13354        }

regards,
dan carpenter