Hi Hans,
Sorry about the late reply. I just returned from vacation.
On Fri, Jul 17, 2020 at 12:04:58PM +0200, Hans de Goede wrote:
> Hi Heikki,
>
> I've been running my personal kernel builds with lockdep enabled
> (more people should do that) and it found an AB BA lock inversion in the
> ucsi driver. This has been introduced by commit 081da1325d35 ("usb: typec:
> ucsi: displayport: Fix a potential race during registration").
>
> The problem is as follows:
>
> AB order:
>
> 1. ucsi_init takes ucsi->ppm_lock (it runs with that locked for the duration of the function)
> 2. usci_init eventually end up calling ucsi_register_displayport, which takes
> ucsi_connector->lock
>
> BA order:
>
> 1. ucsi_handle_connector_change work is started, takes ucsi_connector->lock
> 2. ucsi_handle_connector_change calls ucsi_send_command which takes ucsi->ppm_lock
>
> I think this can be fixed by doing the following:
>
> a. Make ucsi_init drop the ucsi->ppm_lock before it starts registering ports; and
> replacing any ucsi_run_command calls after this point with ucsi_send_command
> (which is a wrapper around run_command taking the lock while handling the command)
>
> b. Move the taking of the ucsi_connector->lock from ucsi_register_displayport into
> ucsi_register_port() to make sure that nothing can touch the connector/port until
> ucsi_register_port() has completed.
>
>
> b. is not stricly necessary but it brings the locking during init more inline
> with locking done during runtime so this seems like a good idea.
Makes sense. So b. it is. Can you prepare the patch for that?
thanks,
--
heikki
