Re: [PATCH PREEMPT_RT] kcov: fix locking splat from kcov_remote_start()

Sebastian Andrzej Siewior
2021-08-10T09:50:39Z

On 2021-08-09 15:59:09 [-0500], Clark Williams wrote:
> Saw the following splat on 5.14-rc4-rt5 with:
…
> Change kcov_remote_lock from regular spinlock_t to raw_spinlock_t so that
> we don't get "sleeping function called from invalid context" on PREEMPT_RT kernel.

I'm not entirely happy with that:
- kcov_remote_start() decouples spin_lock_irq() and does local_irq_save()
+ spin_lock() which shouldn't be done as per
Documentation/locking/locktypes.rst
I would prefer to see the local_irq_save() replaced by
local_lock_irqsave() so we get a context on what is going on.

- kcov_remote_reset() has a kfree() with that irq-off lock acquired.

- kcov_remote_add() has a kmalloc() and is invoked with that irq-off
lock acquired.

- kcov_remote_area_put() uses INIT_LIST_HEAD() for no reason (just
happen to notice).

- kcov_remote_stop() does local_irq_save() + spin_lock(&kcov->lock);.
This should also create a splat.

- With lock kcov_remote_lock acquired there is a possible
hash_for_each_safe() and list_for_each() iteration. I don't know what
the limits are here but with a raw_spinlock_t it will contribute to
the maximal latency.

> Signed-off-by: Clark Williams <williams@redhat.com>

Sebastian