Re: [PATCH 2/8] i3c: master: Serialize i3c_set_hotjoin() with the maintenance lock
Adrian Hunter <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 12/05/2026 22:42, David Nyström wrote: > > > On Tue, 12 May 2026, Frank Li wrote: > >> On Tue, May 12, 2026 at 03:17:26PM +0300, Adrian Hunter wrote: >>> i3c_set_hotjoin() dispatches the controller's enable_hotjoin() or >>> disable_hotjoin() op and updates master->hotjoin under >>> i3c_bus_normaluse_lock(). That lock is a read-side acquisition of >>> bus->lock (down_read()), so it does not exclude concurrent callers. >>> >>> The hotjoin sysfs attribute can be opened multiple times, and writes >>> through different opens are not serialized. Two concurrent writers >>> to "hotjoin" can therefore race in i3c_set_hotjoin(), with the >>> controller op and the master->hotjoin store from one call interleaving >>> with the other. The hardware enable/disable state and the value reported >>> by hotjoin_show() can end up out of sync. >>> >>> Take i3c_bus_maintenance_lock() instead. Toggling Hot Join enable >>> changes bus state and is conceptually a maintenance operation, so the >>> write-side acquisition of bus->lock is the appropriate lock and >>> serializes concurrent callers against each other and against other >>> maintenance operations. >> >> It should be bug fix, add fix tag here. > > Agreed, Fixes: "i3c: master: Add sysfs option to rescan bus via entdaa" > Is this series headed for 7.1-rc3 ? if not, its probably wise to revert the sysfs addition from 7.1-rc "i3c: master: Add sysfs option to rescan bus via entdaa" added "do_daa". "hotjoin" is a different sysfs attribute. > >> Frank >>> >>> Signed-off-by: Adrian Hunter <[email protected]> >>> --- >>> drivers/i3c/master.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c >>> index ab11e2d79aab..38ffc8713167 100644 >>> --- a/drivers/i3c/master.c >>> +++ b/drivers/i3c/master.c >>> @@ -649,7 +649,7 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable) >>> return ret; >>> } >>> >>> - i3c_bus_normaluse_lock(&master->bus); >>> + i3c_bus_maintenance_lock(&master->bus); >>> >>> if (enable) >>> ret = master->ops->enable_hotjoin(master); >>> @@ -659,7 +659,7 @@ static int i3c_set_hotjoin(struct i3c_master_controller *master, bool enable) >>> if (!ret) >>> master->hotjoin = enable; >>> >>> - i3c_bus_normaluse_unlock(&master->bus); >>> + i3c_bus_maintenance_unlock(&master->bus); >>> >>> if ((enable && ret) || (!enable && !ret) || master->rpm_ibi_allowed) >>> i3c_master_rpm_put(master); >>> -- >>> 2.51.0 >>> >> >> -- >> linux-i3c mailing list >> [email protected] >> http://lists.infradead.org/mailman/listinfo/linux-i3c >> -- linux-i3c mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-i3c