Re: [PATCH v2 2/4] i2c: Replace list-based mechanism for handling auto-detected clients

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

Hi Heiner,

sorry for the slow response. I am on the road for two weeks now which
doesn't leave a lot of review time.

The good (or bad?) news is that I finally found why I had the feeling of
"something still missing" from this very interesting approach.

> -	/* Tell drivers about this removal */
> -	mutex_lock(&core_lock);
> -	bus_for_each_drv(&i2c_bus_type, NULL, adap,
> -			       __process_removed_adapter);
> -	mutex_unlock(&core_lock);

You remove using the lock here...

> -	i2c_for_each_dev(driver, __process_removed_driver);
> +	/* Satisfy __must_check, function can't fail */
> +	if (driver_for_each_device(&driver->driver, NULL, NULL,

... and here, because i2c_for_each_dev() utilizes the lock as well. This
is, you open a race window for deleting clients via removing the driver
and removing the adapter at the "same" time.

The obvious solution is to use the lock also when removing clients in
i2c_del_adapter(). But this needs careful thinking about potential side
effects.

Makes sense so far?

All the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]