Re: [PATCH 2/8] i3c: master: Serialize i3c_set_hotjoin() with the maintenance lock
David Nyström <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 13 May 2026, Adrian Hunter wrote: > 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. Replied to wrong patch, correct patch commented now. >> >>> 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