Re: Performance regression in GPIOLIB with SRCU when using the user-space ABI in a *wrong* way

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Mon, May 6, 2024 at 3:55 PM Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
>
> On Mon, May 06, 2024 at 02:32:57PM +0200, Bartosz Golaszewski wrote:
>>
> > The offending kernel code looks like this:
> >
> >     old = rcu_replace_pointer(desc->label, new, 1);
> >     synchronize_srcu(&desc->srcu);
> >     kfree_const(old);
> >
> > I was wondering if we even have to synchronize here? The corresponding
> > read-only sections call srcu_dereference(desc->label, &desc->srcu).
> > Would it be enough to implement kfree_const_rcu() and use it here
> > without synchronizing? Would the read-only users correctly see that
> > last dereferenced address still points to valid memory until they all
> > release the lock and the memory would only then be freed? Is my
> > understanding of kfree_rcu() correct?
>
> It looks like kfree_const() just does a kfree(), so why not use
> call_srcu() to replace the above calls to synchronize_srcu() and
> kfree_const()?
>
> Something like this:
>
>         if (!is_kernel_rodata((unsigned long)(old)))
>                 call_srcu(&desc->srcu, &desc->rh, gpio_cb);
>
> This requires adding an rcu_head field named "rh" to the structure
> referenced by "desc" and creating a gpio_cb() wrapper function:
>
> static void connection_release(struct rcu_head *rhp)
> {
>         struct beats_me *bmp = container_of(rhp, struct beats_me, rh);
>
>         kfree(bmp);
> }
>
> I could not find the code, so I don't know what "beats_me" above
> should be replaced with.
>
> Would that work?

Thanks, this looks like a potential solution but something's not clear
for me. What I want to free here is "old". However, its address is a
field in struct beats_me and it's replaced (using
rcu_replace_pointer()) before scheduling the free. When
connection_release() will eventually be called, I think the address
under bmp->label will be the new one? How would I pass some arbitrary
user data to this callback (if at all possible?).

Bart