Moi, On Fri, Mar 03, 2023 at 05:52:49PM +0200, Tomi Valkeinen wrote: > The new subdev state code has a possible deadlock scenario during link > validation when the pipeline contains subdevs that support state and > that do not support state. > > The current code locks the states of the subdevs on both ends of the > link when starting the link validation, locking the sink side first, > then the source. If either (or both) of the subdevs does not support > state, nothing is done for that subdev at this point, and instead the > locking is handled the old way, i.e. the subdev's ops do the locking > internally. > > The issue arises when the sink doesn't support state, but source does, > so the validation code locks the source for the duration of the > validation, and then the sink is locked only when the get_fmt op is > called. So lockdep sees the source locked first, then the sink. > > Later, when the streaming is started, the sink's s_stream op is called, > which probably takes the subdev's lock. The op then calls the source's > s_stream, which takes the source's lock. So, the sink is locked first, > then the source. > > Note that link validation and stream starting is not done at the same > time, so an actual deadlock should never happen. However, it's still a > clear bug. > > Fix this by locking the subdev states only if both subdevs support > state. In other words, we have two scenarios: > > 1. Both subdevs support state. Lock sink first, then source, and keep > the locks while validating the link. > 2. At least one of the subdevs do not support state. Take the lock only > for the duration of the operation (get_fmt or looking at the > routing), and release after the op is done. > > Obviously 1. is better, as we have a more consistent view of the states > of the subdevs during validation. 2. is how it has been so far, so it's > no worse than this used to be. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx> Thanks for the patch, Tomi! I'll add this to my next fixes PR, probably tomorrow, unless there are objections. I don't think this is a false positive: the same locks may well be used for the same purpose in different video nodes. -- Terveisin, Sakari Ailus
