* H. Nikolaus Schaller hns@goldelico.com [180616 11:10]:
There is also good news: the selectors are now assigned in strict sequence. This is a big improvement:
OK thanks for testing.
If slots 17 and 18 do not really exist (or are deleted after failed probing), there is a NULL return. Which triggers the strcmp(NULL), because the strcmp is not guarded against non-existing slots in the radix tree.
So a simple workaround could be:
if (gname && !strcmp(gname, pin_group)) {But I think the fundamental problem is that the same driver assigns multiple slots if probing is deferred.
Therefore, I tried to find out more why this happens. Here are some more observations by studying the code and adding a dump_stack() inside pinctrl_generic_add_group():
- the records stored in the radix tree are allocated through devm_kzalloc() within pinctrl_generic_add_group().
- I have not found a mechanism that removes them from the radix tree if they are devm freed which IMHO happens if the probe fails (and all devm objects are rewound).
- I could not observe calls to pinctrl_generic_remove_group()
- this means a stale pointer to the group_desc is still stored in the radix tree if driver probing fails
- scanning through the radix tree for a matching gname in pinctrl_get_group_selector() accesses this stale radix tree entry. It has either been overwritten by something else - or contains a dangling pointer for group->name. This explains the randomness of the problem but that it is also repeatable to some extent. For a pure race of some 100 instructions it happens IMHO too often.
- the pinctrl_generic_add_group() is called from create_pinctrl() through pinctrl_dt_to_map() and pcs_dt_node_to_map(), i.e. before the pinctrl_maps_mutex is locked in create_pinctrl(). This means that pinctrl_generic_add_group() is not locked in this case.
I hope this helps to pinpoint and solve the remaining bugs.
Yes sounds like that's where things still go wrong. I'll take a look if it makes sense to assign the selector from the pin controller driver instead or just ignoring the holes.
I'll try to debug the devm issue too.
Regards,
Tony