Possible deadlock scenario (on reboot):
rk3x_i2c_xfer_common(polling)
-> rk3x_i2c_wait_xfer_poll()
-> rk3x_i2c_irq(0, i2c);
--> spin_lock(&i2c->lock);
...
<rk3x i2c interrupt>
-> rk3x_i2c_irq(0, i2c);
--> spin_lock(&i2c->lock); (deadlock here)
Store the IRQ number and disable/enable it around the polling transfer.
This patch has been tested on NanoPC-T4.
Signed-off-by: Jensen Huang <[email protected]>
---
drivers/i2c/busses/i2c-rk3x.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index a044ca0c35a1..94514637c3bd 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -200,6 +200,7 @@ struct rk3x_i2c {
struct clk *clk;
struct clk *pclk;
struct notifier_block clk_rate_nb;
+ int irq;
/* Settings */
struct i2c_timings t;
@@ -1087,13 +1088,18 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
spin_unlock_irqrestore(&i2c->lock, flags);
- rk3x_i2c_start(i2c);
-
if (!polling) {
+ rk3x_i2c_start(i2c);
+
timeout = wait_event_timeout(i2c->wait, !i2c->busy,
msecs_to_jiffies(WAIT_TIMEOUT));
} else {
+ disable_irq(i2c->irq);
+ rk3x_i2c_start(i2c);
+
timeout = rk3x_i2c_wait_xfer_poll(i2c);
+
+ enable_irq(i2c->irq);
}
spin_lock_irqsave(&i2c->lock, flags);
@@ -1310,6 +1316,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
return ret;
}
+ i2c->irq = irq;
+
platform_set_drvdata(pdev, i2c);
if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) {
--
2.42.0
Am Mittwoch, 6. Dezember 2023, 10:06:40 CET schrieb Jensen Huang:
> Possible deadlock scenario (on reboot):
> rk3x_i2c_xfer_common(polling)
> -> rk3x_i2c_wait_xfer_poll()
> -> rk3x_i2c_irq(0, i2c);
> --> spin_lock(&i2c->lock);
> ...
> <rk3x i2c interrupt>
> -> rk3x_i2c_irq(0, i2c);
> --> spin_lock(&i2c->lock); (deadlock here)
>
> Store the IRQ number and disable/enable it around the polling transfer.
> This patch has been tested on NanoPC-T4.
>
> Signed-off-by: Jensen Huang <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
> ---
> drivers/i2c/busses/i2c-rk3x.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> index a044ca0c35a1..94514637c3bd 100644
> --- a/drivers/i2c/busses/i2c-rk3x.c
> +++ b/drivers/i2c/busses/i2c-rk3x.c
> @@ -200,6 +200,7 @@ struct rk3x_i2c {
> struct clk *clk;
> struct clk *pclk;
> struct notifier_block clk_rate_nb;
> + int irq;
>
> /* Settings */
> struct i2c_timings t;
> @@ -1087,13 +1088,18 @@ static int rk3x_i2c_xfer_common(struct i2c_adapter *adap,
>
> spin_unlock_irqrestore(&i2c->lock, flags);
>
> - rk3x_i2c_start(i2c);
> -
> if (!polling) {
> + rk3x_i2c_start(i2c);
> +
> timeout = wait_event_timeout(i2c->wait, !i2c->busy,
> msecs_to_jiffies(WAIT_TIMEOUT));
> } else {
> + disable_irq(i2c->irq);
> + rk3x_i2c_start(i2c);
> +
> timeout = rk3x_i2c_wait_xfer_poll(i2c);
> +
> + enable_irq(i2c->irq);
> }
>
> spin_lock_irqsave(&i2c->lock, flags);
> @@ -1310,6 +1316,8 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> return ret;
> }
>
> + i2c->irq = irq;
> +
> platform_set_drvdata(pdev, i2c);
>
> if (i2c->soc_data->calc_timings == rk3x_i2c_v0_calc_timings) {
>
Hi Jensen,
On Wed, Dec 06, 2023 at 05:06:40PM +0800, Jensen Huang wrote:
> Possible deadlock scenario (on reboot):
> rk3x_i2c_xfer_common(polling)
> -> rk3x_i2c_wait_xfer_poll()
> -> rk3x_i2c_irq(0, i2c);
> --> spin_lock(&i2c->lock);
> ...
> <rk3x i2c interrupt>
> -> rk3x_i2c_irq(0, i2c);
> --> spin_lock(&i2c->lock); (deadlock here)
>
> Store the IRQ number and disable/enable it around the polling transfer.
> This patch has been tested on NanoPC-T4.
>
> Signed-off-by: Jensen Huang <[email protected]>
Looks good,
Reviewed-by: Andi Shyti <[email protected]>
Thanks,
Andi
Hi Jensen,
kernel test robot noticed the following build warnings:
[auto build test WARNING on rockchip/for-next]
[also build test WARNING on linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jensen-Huang/i2c-rk3x-fix-potential-spinlock-recursion-on-poll/20231206-170807
base: https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git for-next
patch link: https://lore.kernel.org/r/20231206090641.18849-1-jensenhuang%40friendlyarm.com
patch subject: [PATCH] i2c: rk3x: fix potential spinlock recursion on poll
config: arc-randconfig-r081-20231206 (https://download.01.org/0day-ci/archive/20231207/[email protected]/config)
compiler: arc-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-rk3x.c:223: warning: Function parameter or member 'irq' not described in 'rk3x_i2c'
vim +223 drivers/i2c/busses/i2c-rk3x.c
c41aa3ce938b68 Max Schwarz 2014-06-11 171
0a6ad2f95f36bf David Wu 2016-05-16 172 /**
0a6ad2f95f36bf David Wu 2016-05-16 173 * struct rk3x_i2c - private data of the controller
0a6ad2f95f36bf David Wu 2016-05-16 174 * @adap: corresponding I2C adapter
0a6ad2f95f36bf David Wu 2016-05-16 175 * @dev: device for this controller
0a6ad2f95f36bf David Wu 2016-05-16 176 * @soc_data: related soc data struct
0a6ad2f95f36bf David Wu 2016-05-16 177 * @regs: virtual memory area
7e086c3fc2df09 David Wu 2016-05-16 178 * @clk: function clk for rk3399 or function & Bus clks for others
7e086c3fc2df09 David Wu 2016-05-16 179 * @pclk: Bus clk for rk3399
0a6ad2f95f36bf David Wu 2016-05-16 180 * @clk_rate_nb: i2c clk rate change notify
0a6ad2f95f36bf David Wu 2016-05-16 181 * @t: I2C known timing information
0a6ad2f95f36bf David Wu 2016-05-16 182 * @lock: spinlock for the i2c bus
0a6ad2f95f36bf David Wu 2016-05-16 183 * @wait: the waitqueue to wait for i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 184 * @busy: the condition for the event to wait for
0a6ad2f95f36bf David Wu 2016-05-16 185 * @msg: current i2c message
0a6ad2f95f36bf David Wu 2016-05-16 186 * @addr: addr of i2c slave device
0a6ad2f95f36bf David Wu 2016-05-16 187 * @mode: mode of i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 188 * @is_last_msg: flag determines whether it is the last msg in this transfer
0a6ad2f95f36bf David Wu 2016-05-16 189 * @state: state of i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 190 * @processed: byte length which has been send or received
0a6ad2f95f36bf David Wu 2016-05-16 191 * @error: error code for i2c transfer
0a6ad2f95f36bf David Wu 2016-05-16 192 */
c41aa3ce938b68 Max Schwarz 2014-06-11 193 struct rk3x_i2c {
c41aa3ce938b68 Max Schwarz 2014-06-11 194 struct i2c_adapter adap;
c41aa3ce938b68 Max Schwarz 2014-06-11 195 struct device *dev;
d032a2eb2e3b9f Julia Lawall 2018-01-02 196 const struct rk3x_i2c_soc_data *soc_data;
c41aa3ce938b68 Max Schwarz 2014-06-11 197
c41aa3ce938b68 Max Schwarz 2014-06-11 198 /* Hardware resources */
c41aa3ce938b68 Max Schwarz 2014-06-11 199 void __iomem *regs;
c41aa3ce938b68 Max Schwarz 2014-06-11 200 struct clk *clk;
7e086c3fc2df09 David Wu 2016-05-16 201 struct clk *pclk;
249051f49907e7 Max Schwarz 2014-11-20 202 struct notifier_block clk_rate_nb;
58bb392dd4c7fc Jensen Huang 2023-12-06 203 int irq;
c41aa3ce938b68 Max Schwarz 2014-06-11 204
c41aa3ce938b68 Max Schwarz 2014-06-11 205 /* Settings */
1ab92956d4aac5 David Wu 2016-03-17 206 struct i2c_timings t;
c41aa3ce938b68 Max Schwarz 2014-06-11 207
c41aa3ce938b68 Max Schwarz 2014-06-11 208 /* Synchronization & notification */
c41aa3ce938b68 Max Schwarz 2014-06-11 209 spinlock_t lock;
c41aa3ce938b68 Max Schwarz 2014-06-11 210 wait_queue_head_t wait;
c41aa3ce938b68 Max Schwarz 2014-06-11 211 bool busy;
c41aa3ce938b68 Max Schwarz 2014-06-11 212
c41aa3ce938b68 Max Schwarz 2014-06-11 213 /* Current message */
c41aa3ce938b68 Max Schwarz 2014-06-11 214 struct i2c_msg *msg;
c41aa3ce938b68 Max Schwarz 2014-06-11 215 u8 addr;
c41aa3ce938b68 Max Schwarz 2014-06-11 216 unsigned int mode;
c41aa3ce938b68 Max Schwarz 2014-06-11 217 bool is_last_msg;
c41aa3ce938b68 Max Schwarz 2014-06-11 218
c41aa3ce938b68 Max Schwarz 2014-06-11 219 /* I2C state machine */
c41aa3ce938b68 Max Schwarz 2014-06-11 220 enum rk3x_i2c_state state;
0a6ad2f95f36bf David Wu 2016-05-16 221 unsigned int processed;
c41aa3ce938b68 Max Schwarz 2014-06-11 222 int error;
c41aa3ce938b68 Max Schwarz 2014-06-11 @223 };
c41aa3ce938b68 Max Schwarz 2014-06-11 224
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki