Hi, On 7/27/20 2:37 PM, Heikki Krogerus wrote:
Hi Hans, Sorry about the late reply. I just returned from vacation.
NP. I hope you've enjoyed your 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?
b. is just a cleanup / extra step on top of a. we need a. to fix the inversion. If you are ok with that I can try to make some time for this... Regards, Hans
