Hi Thierry,

On Tue, Mar 12, 2019 at 12:55 PM Thierry Reding
<[email protected]> wrote:
> On Tue, Mar 12, 2019 at 12:16:34PM +0900, Yoshihiro Shimoda wrote:
> > From: Phong Hoang <[email protected]>
> >
> > This patch fixes deadlock warning if removing PWM device
> > when CONFIG_PROVE_LOCKING is enabled.
> >
> > This issue can be reproceduced by the following steps on
> > the R-Car H3 Salvator-X board if the backlight is disabled:
> >
> >  # cd /sys/class/pwm/pwmchip0
> >  # echo 0 > export
> >  # ls
> >  device  export  npwm  power  pwm0  subsystem  uevent  unexport
> >  # cd device/driver
> >  # ls
> >  bind  e6e31000.pwm  uevent  unbind
> >  # echo e6e31000.pwm > unbind
> >
> > [   87.659974] ======================================================
> > [   87.666149] WARNING: possible circular locking dependency detected
> > [   87.672327] 5.0.0 #7 Not tainted
> > [   87.675549] ------------------------------------------------------
> > [   87.681723] bash/2986 is trying to acquire lock:
> > [   87.686337] 000000005ea0e178 (kn->count#58){++++}, at: 
> > kernfs_remove_by_name_ns+0x50/0xa0
> > [   87.694528]
> > [   87.694528] but task is already holding lock:
> > [   87.700353] 000000006313b17c (pwm_lock){+.+.}, at: 
> > pwmchip_remove+0x28/0x13c
> > [   87.707405]
> > [   87.707405] which lock already depends on the new lock.
> > [   87.707405]
> > [   87.715574]
> > [   87.715574] the existing dependency chain (in reverse order) is:
> > [   87.723048]
> > [   87.723048] -> #1 (pwm_lock){+.+.}:
> > [   87.728017]        __mutex_lock+0x70/0x7e4
> > [   87.732108]        mutex_lock_nested+0x1c/0x24
> > [   87.736547]        pwm_request_from_chip.part.6+0x34/0x74
> > [   87.741940]        pwm_request_from_chip+0x20/0x40
> > [   87.746725]        export_store+0x6c/0x1f4
> > [   87.750820]        dev_attr_store+0x18/0x28
> > [   87.754998]        sysfs_kf_write+0x54/0x64
> > [   87.759175]        kernfs_fop_write+0xe4/0x1e8
> > [   87.763615]        __vfs_write+0x40/0x184
> > [   87.767619]        vfs_write+0xa8/0x19c
> > [   87.771448]        ksys_write+0x58/0xbc
> > [   87.775278]        __arm64_sys_write+0x18/0x20
> > [   87.779721]        el0_svc_common+0xd0/0x124
> > [   87.783986]        el0_svc_compat_handler+0x1c/0x24
> > [   87.788858]        el0_svc_compat+0x8/0x18
> > [   87.792947]
> > [   87.792947] -> #0 (kn->count#58){++++}:
> > [   87.798260]        lock_acquire+0xc4/0x22c
> > [   87.802353]        __kernfs_remove+0x258/0x2c4
> > [   87.806790]        kernfs_remove_by_name_ns+0x50/0xa0
> > [   87.811836]        remove_files.isra.1+0x38/0x78
> > [   87.816447]        sysfs_remove_group+0x48/0x98
> > [   87.820971]        sysfs_remove_groups+0x34/0x4c
> > [   87.825583]        device_remove_attrs+0x6c/0x7c
> > [   87.830197]        device_del+0x11c/0x33c
> > [   87.834201]        device_unregister+0x14/0x2c
> > [   87.838638]        pwmchip_sysfs_unexport+0x40/0x4c
> > [   87.843509]        pwmchip_remove+0xf4/0x13c
> > [   87.847773]        rcar_pwm_remove+0x28/0x34
> > [   87.852039]        platform_drv_remove+0x24/0x64
> > [   87.856651]        device_release_driver_internal+0x18c/0x21c
> > [   87.862391]        device_release_driver+0x14/0x1c
> > [   87.867175]        unbind_store+0xe0/0x124
> > [   87.871265]        drv_attr_store+0x20/0x30
> > [   87.875442]        sysfs_kf_write+0x54/0x64
> > [   87.879618]        kernfs_fop_write+0xe4/0x1e8
> > [   87.884055]        __vfs_write+0x40/0x184
> > [   87.888057]        vfs_write+0xa8/0x19c
> > [   87.891887]        ksys_write+0x58/0xbc
> > [   87.895716]        __arm64_sys_write+0x18/0x20
> > [   87.900154]        el0_svc_common+0xd0/0x124
> > [   87.904417]        el0_svc_compat_handler+0x1c/0x24
> > [   87.909289]        el0_svc_compat+0x8/0x18
>
> I'm not sure I understand this correctly. The above looks like pwm_lock
> is held as part of the syscall writing 0 to the export attribute and the
> second callchain looks like it's originating from the write to the
> unbind attribute for the driver. But pwm_request_from_chip() should have
> already released the lock before it returned, so how can the above
> situation even happen?

Note that it says "_possible_ circular locking dependency detected".
AFAIU, this does not mean that the above two callchains actually did
happen at the same time.

Lockdep keeps tracks of the order in which locks are taken.  If it
notices that one chain takes locks in one order, and a second chain
takes those locks in a different order, it prints a warning, as this
could cause a deadlock if/when the two callchains would ever happen
at the same time.

Note that there may be other reasons, outside the view of lockdep, which
guarantee this cannot actually happen...

So far my understanding...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to