* H. Nikolaus Schaller hns@goldelico.com [180618 18:33]:
So code just needs group cleanup on failed probing and fixing the mutex around pinctrl_generic_add_group().
I think we need the mutex because a race still can happen when create_pinctrl() is calling pcs_dt_node_to_map() and pinctrl_generic_add_group() w/o being locked on pinctrl_maps_mutex.
The race I suspect is that two drivers are trying to insert the same name and may come both to the conclusion that it does not yet exist. And both insert into the radix tree.
The window of risk is small though... It is in pinctrl_generic_add_group() between calling pinctrl_generic_group_name_to_selector() and radix_tree_insert() so we probably won't see it in real hardware tests.
Hmm but that race should be already fixed with mutex held by the pin controller drivers with these fixes? Or am I missing something still?
Hm. Maybe we refer to a different mutex?
Yes I think that's the case, you're talking about a different mutex here :)
I had seen the call sequence
create_pinctrl()-> pinctrl_dt_to_map() -> pcs_dt_node_to_map() -> pinctrl_generic_add_group()
w/o any lock inside.
There is a mutex_lock(&pinctrl_maps_mutex); in create_pinctrl(), but locked after that.
Or is there a lock outside of create_pinctrl()?
If I look into the stack dumps, call nesting is
driver_probe_device() -> pinctrl_bind_pins() -> devm_pinctrl_get() -> create_pinctrl()
They all do no locking.
Maybe I am missing something.
Can you please post a patch for that as you already have it debugged? That's easier to understand than reading a verbal patch :)
Regards,
Tony