Hi Michael, Rodolfo, David, I found a race condition in function i2c_new_device(): /* Check for address business */ status = i2c_check_addr_busy(adap, client->addr); if (status) goto out_err; client->dev.parent = &client->adapter->dev; client->dev.bus = &i2c_bus_type; client->dev.type = &i2c_client_type; #ifdef CONFIG_OF client->dev.of_node = info->of_node; #endif dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap), client->addr); status = device_register(&client->dev); if (status) goto out_err; i2c_check_addr_busy() relies on the device tree maintained by the driver core to figure out whether an address is busy. This tree is updated only when device_register() returns. This means that, in the event where i2c_new_device() is run twice in parallel for the same address on related I2C segments, both may pass the i2c_check_addr_busy() check, and then either one of the device_register() calls will fail with the driver code complaining about duplicate bus_id (which could already happen in previous kernels, as the race condition isn't new) or both will succeed and we end up with a parent and child I2C segments with an i2c client at the same address - which is wrong. This last case is new, and possible only since multiplexing support was added. I was about to add a new mutex protecting this code flow, but suddenly realized that it might deadlock when registering i2c multiplexer devices. For such devices, the probe function will register i2c adapters, on which i2c_scan_static_board_info() may be called, which in turn will call i2c_new_device(). So anyone describing multiplexed topologies as static board information will hit the deadlock. At this point, I am rather clueless if and how this problem should be solved, so I would welcome suggestions. Maintaining our own list of busy addresses would probably work, however we tried hard to get rid of duplicate housekeeping work in the past few years and I would be very unhappy if we have to go backwards. Another possibility is to ignore the issue and consider this check as best effort. It can only fail if the provided device information is incorrect, in which case it will have to be fixed anyway. In that case, we can discuss whether it makes sense to keep i2c_check_addr_busy(), if we know it's not 100% reliable, we might as well do without it. The only benefit I see in keeping it is that it should make it easier to spot the error and correct it. Opinions? Thanks, -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
