Re: [PATCH] revocable: Remove redundant synchronize_srcu() call

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Wed, Jan 21, 2026 at 03:18:12PM +0000, Bartosz Golaszewski wrote:
> On Wed, 21 Jan 2026 13:34:15 +0100, Tzung-Bi Shih <tzungbi@kernel.org> said:
> > On Wed, Jan 21, 2026 at 11:38:18AM +0100, Bartosz Golaszewski wrote:
> >> On Wed, Jan 21, 2026 at 5:03 AM Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> >> >
> >> > When allocating a revocable provider via revocable_provider_alloc(),
> >> > there is no revocable consumers (i.e., RCU readers) yet.  Remove the
> >> > redundant synchronize_srcu() call to save cycles.
> >> >
> >> > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> >> > ---
> >> > To address the performance impact reported in
> >> > https://lore.kernel.org/linux-gpio/CAMRc=McrFa42mNWmZtD1HKKKZ+USUKpQAAME50wbfxPM7L72gA@mail.gmail.com/.
> >> >
> >> >  drivers/base/revocable.c | 1 -
> >> >  1 file changed, 1 deletion(-)
> >> >
> >> > diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c
> >> > index f6cece275aac..b068e18a847d 100644
> >> > --- a/drivers/base/revocable.c
> >> > +++ b/drivers/base/revocable.c
> >> > @@ -99,7 +99,6 @@ struct revocable_provider *revocable_provider_alloc(void *res)
> >> >
> >> >         init_srcu_struct(&rp->srcu);
> >> >         rcu_assign_pointer(rp->res, res);
> >> > -       synchronize_srcu(&rp->srcu);
> >> >         kref_init(&rp->kref);
> >> >
> >> >         return rp;
> >> > --
> >> > 2.52.0.457.g6b5491de43-goog
> >> >
> >>
> >> Do you have an up-to-date integration branch with all the series and
> >> fixes you posted that I could use for testing with GPIO?
> >
> > Please use
> > https://git.kernel.org/pub/scm/linux/kernel/git/tzungbi/chrome-platform.git/log/?h=gpio_rev
> > for the purpose which is based on driver-core-next and gpio/for-current
> > branches.
> >
> 
> With this branch, I'm seeing the following KASAN splat when running libgpiod
> tests:
> 
> ==================================================================
> BUG: KASAN: use-after-free in notifier_call_chain+0x222/0x270
> Read of size 8 at addr ffff888103ce6490 by task gpiod-test/479
> 
> CPU: 2 UID: 0 PID: 479 Comm: gpiod-test Not tainted
> 6.19.0-rc6-yocto-standard+ #2 PREEMPT(full)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x63/0x90
>  print_report+0x153/0x4c7
>  ? __virt_addr_valid+0x225/0x4c0
>  ? kasan_addr_to_slab+0x11/0x80
>  kasan_report+0x103/0x140
>  ? notifier_call_chain+0x222/0x270
>  ? notifier_call_chain+0x222/0x270
>  __asan_report_load8_noabort+0x18/0x20
>  notifier_call_chain+0x222/0x270
>  blocking_notifier_call_chain+0x70/0xb0
>  gpiolib_cdev_unregister+0x5a/0x70
>  gpiochip_remove+0x48e/0x5a0
>  ? __kasan_slab_free+0x84/0xa0
>  devm_gpio_chip_release+0x12/0x20

Even though I can't reproduce the error for now, by inspecting the calling
stack, I think what was happening:

| Thread A - removing the chip       | Thread B - closing the FD        |
|                                    |                                  |
| gpiochip_remove()                  |                                  |
|   revocable_provider_revoke()      |                                  |
|                                    | The .close() finds the           |
|                                    | gpio_chip is gone, skip to       |
|                                    | unregister notifier...(1).       |
|                                    |                                  |
|                                    | The .close() frees the context.  |
|                                    | E.g., kfree(cdev);               |
|   gcdev_unregister()               |                                  |
|     gpiolib_cdev_unregister()      |                                  |
|       It notifies the subscribers  |                                  |
|       about the GPIO chip being    |                                  |
|       unregistered.  And it tries  |                                  |
|       to access `cdev` which has   |                                  |
|       been freed...(2).            |                                  |


(1) REVOCABLE_TRY_ACCESS_SCOPED(cdev->chip_rev, gc) {
	    if (!gc)
		    break;
	    gdev = gc->gpiodev;

	    blocking_notifier_chain_unregister(&gdev->device_notifier,
			    &cdev->device_unregistered_nb);

	    scoped_guard(write_lock_irqsave, &gdev->line_state_lock)
		    raw_notifier_chain_unregister(&gdev->line_state_notifier,
				    &cdev->lineinfo_changed_nb);
    }

(2) void gpiolib_cdev_unregister(struct gpio_device *gdev)
    {
	    destroy_workqueue(gdev->line_state_wq);
	    cdev_device_del(&gdev->chrdev, &gdev->dev);
	    blocking_notifier_call_chain(&gdev->device_notifier, 0, NULL);
    }

This is a separate issue.  I'd find a way to reproduce it and will see
how to fix it.