Re: [PATCH 1/1] rcu/tree: Drop the lock before entering to page allocator

From: Paul E. McKenney
Date: Thu Jul 16 2020 - 11:20:33 EST


On Thu, Jul 16, 2020 at 04:14:21PM +0200, Sebastian Andrzej Siewior wrote:
> On 2020-07-15 15:14:49 [-0700], Paul E. McKenney wrote:
> >
> > My concern is that some critical bug will show up at some point
> > that requires double-argument kfree_rcu() be invoked while holding
> > a raw spinlock. (Single-argument kfree_rcu() must sometimes invoke
> > synchronize_rcu(), so it can never be invoked in any state forbidding
> > invoking schedule().)
>
> So you are saying as of today we are good but in near future the
> following
> synchronize_rcu() -> kfree_rcu()
>
> may be needed?

You lost me on this one. I am instead concerned that something like this
might be needed on short notice:

raw_spin_lock(&some_lock);
kfree_rcu(some_pointer, some_field_offset);

In contrast, single-argument kfree_rcu() cannot be invoked from any
environment where synchronize_rcu() cannot be invoked.

> > Yes, dropping to a plain spinlock would be simple in the here and now,
> > but experience indicates that it is only a matter of time, and that when
> > that time comes it will come as an emergency.
>
> Hmmm.

I point out the call_rcu() experience.

> > One approach would be to replace the "IS_ENABLED(CONFIG_PREEMPT_RT)"
> > with some sort of check for being in a context where spinlock acquisition
> > is not legal. What could be done along those lines?
>
> I would rethink the whole concept how this is implemented now and give
> it another try. The code does not look pretty and is looking
> complicated. The RT covering of this part then just added a simple
> return because nothing else seemed to be possible. This patch here
> looks like another duct tape attempt to avoid a warning.

In addition to the possibility of invocation from BH?

Thanx, Paul