> -----Original Message----- > From: Marek Vasut <marex@xxxxxxx> > Sent: Monday, June 29, 2020 4:49 AM > To: Raviteja Narayanam <rna@xxxxxxxxxx>; linux-i2c@xxxxxxxxxxxxxxx > Cc: Michal Simek <michals@xxxxxxxxxx>; Shubhrajyoti Datta > <shubhraj@xxxxxxxxxx>; Wolfram Sang <wsa@xxxxxxxxxx> > Subject: Re: [PATCH 1/5] i2c: xiic: Fix broken locking on tx_msg > > On 6/26/20 2:11 PM, Raviteja Narayanam wrote: > > Hi, > > [...] > > >> @@ -699,14 +708,7 @@ static int xiic_xfer(struct i2c_adapter *adap, > >> struct i2c_msg *msgs, int num) > >> if (err < 0) > >> return err; > >> > >> - err = xiic_busy(i2c); > >> - if (err) > >> - goto out; > >> - > >> - i2c->tx_msg = msgs; > >> - i2c->nmsgs = num; > > > > On an SMP system with multiple i2c-transfer command scripts running, the > above critical section is expected to cause serious trouble overwriting the > previous msg pointers. > > But that's not happening as the i2c-core is having a lock at adapter level > inside i2c-core-base.c (rt_mutex_lock_nested). > > So, the race condition between different threads is not possible. They are all > serialized by the locking in i2c-core. > > > > Although no issues are seen in the tests, the contention within the driver is > still possible (isr vs xiic_xfer), if there is a spurious interrupt. And this patch is > needed in that case. > The contention happens between the threaded interrupt handler > xiic_process() and this xiic_xfer() function. While you can not have > xiic_xfer() running on two CPU cores at the same time, you can have > xiic_xfer() running on one CPU core and xiic_process() on another CPU core, > and that will lead to problems. Yes, this patch is needed for that case. > > [...]
