Hi,
Since bae1d3a05a8b, i2c transfers are non-atomic if preemption is disabled. However, non-atomic i2c transfers require preemption (e.g. in wait_for_completion() while waiting for the DMA).
panic() calls preempt_disable_notrace() before calling emergency_restart(). Therefore, if an i2c device is used for the restart, the xfer should be atomic. This avoids warnings like:
[ 12.667612] WARNING: CPU: 1 PID: 1 at kernel/rcu/tree_plugin.h:318 rcu_note_context_switch+0x33c/0x6b0 [ 12.676926] Voluntary context switch within RCU read-side critical section! ... [ 12.742376] schedule_timeout from wait_for_completion_timeout+0x90/0x114 [ 12.749179] wait_for_completion_timeout from tegra_i2c_wait_completion+0x40/0x70 ... [ 12.994527] atomic_notifier_call_chain from machine_restart+0x34/0x58 [ 13.001050] machine_restart from panic+0x2a8/0x32c
Use !preemptible() instead, which is basically the same check as pre-v5.2.
Fixes: bae1d3a05a8b ("i2c: core: remove use of in_atomic()") Cc: stable@vger.kernel.org # v5.2+ Suggested-by: Dmitry Osipenko dmitry.osipenko@collabora.com Acked-by: Wolfram Sang wsa@kernel.org Reviewed-by: Dmitry Osipenko dmitry.osipenko@collabora.com Tested-by: Nishanth Menon nm@ti.com Signed-off-by: Benjamin Bara benjamin.bara@skidata.com
drivers/i2c/i2c-core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.h b/drivers/i2c/i2c-core.h index 1247e6e6e975..05b8b8dfa9bd 100644 --- a/drivers/i2c/i2c-core.h +++ b/drivers/i2c/i2c-core.h @@ -29,7 +29,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources, */ static inline bool i2c_in_atomic_xfer_mode(void) {
- return system_state > SYSTEM_RUNNING && irqs_disabled();
- return system_state > SYSTEM_RUNNING && !preemptible();
With preemption disabled, this boils down to return system_state > SYSTEM_RUNNING (&& !0)
and will then generate a backtrace splash on each reboot on our board:
# reboot -f [ 12.687169] No atomic I2C transfer handler for 'i2c-0' [ 12.692313] WARNING: CPU: 6 PID: 275 at drivers/i2c/i2c-core.h:40 i2c_smbus_xfer+0x100/0x118 [ 12.700745] Modules linked in: [ 12.703788] CPU: 6 PID: 275 Comm: reboot Not tainted 6.7.0-rc6-next-20231222+ #2494 [ 12.711431] Hardware name: Kontron 3.5"-SBC-i1200 (DT) [ 12.716555] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 12.723504] pc : i2c_smbus_xfer+0x100/0x118 [ 12.727674] lr : i2c_smbus_xfer+0x100/0x118 [ 12.731844] sp : ffff80008389b7c0 [ 12.735146] x29: ffff80008389b7c0 x28: ffff0000c561b4c0 x27: ffff0000c2b06400 [ 12.742268] x26: ffff0000c65aec4a x25: 000000000000000b x24: 0000000000000001 [ 12.749389] x23: 0000000000000000 x22: 0000000000000064 x21: 0000000000000008 [ 12.756510] x20: ffff80008389b836 x19: ffff0000c2dda080 x18: ffffffffffffffff [ 12.763631] x17: ffff800080813f48 x16: ffff80008081bd38 x15: 0730072d07630732 [ 12.770752] x14: 0769072707200772 x13: 0730072d07630732 x12: 0769072707200772 [ 12.777873] x11: 0720072007200720 x10: ffff8000827dc828 x9 : ffff80008012e154 [ 12.784994] x8 : 00000000ffffefff x7 : ffff8000827dc828 x6 : 80000000fffff000 [ 12.792116] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 12.799236] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000c561b4c0 [ 12.806359] Call trace: [ 12.808793] i2c_smbus_xfer+0x100/0x118 [ 12.812616] i2c_smbus_read_i2c_block_data+0x60/0xb0 [ 12.817569] mt6360_regmap_read+0xa4/0x1b8 [ 12.821654] _regmap_raw_read+0xfc/0x270 [ 12.825566] _regmap_bus_read+0x4c/0x90 [ 12.829389] _regmap_read+0x6c/0x168 [ 12.832953] _regmap_update_bits+0xf8/0x150 [ 12.837124] regmap_update_bits_base+0x6c/0xa8 [ 12.841555] regulator_disable_regmap+0x48/0x68 [ 12.846073] _regulator_do_disable+0x130/0x1e8 [ 12.850504] _regulator_disable+0x154/0x1d8 [ 12.854676] regulator_disable+0x44/0x90 [ 12.858587] mmc_regulator_set_ocr+0x8c/0x118 [ 12.862933] msdc_ops_set_ios+0x3f4/0x938 [ 12.866932] mmc_set_initial_state+0x90/0xa8 [ 12.871191] mmc_power_off+0x40/0x68 [ 12.874754] _mmc_sd_suspend+0x5c/0x190 [ 12.878577] mmc_sd_suspend+0x20/0x78 [ 12.882227] mmc_bus_shutdown+0x48/0x90 [ 12.886051] device_shutdown+0x134/0x298 [ 12.889961] kernel_restart+0x48/0xd0 [ 12.893613] __do_sys_reboot+0x1e0/0x278 [ 12.897523] __arm64_sys_reboot+0x2c/0x40 [ 12.901519] invoke_syscall+0x50/0x128 [ 12.905257] el0_svc_common.constprop.0+0x48/0xf0 [ 12.909950] do_el0_svc+0x24/0x38 [ 12.913253] el0_svc+0x38/0xd8 [ 12.916296] el0t_64_sync_handler+0x100/0x130 [ 12.920639] el0t_64_sync+0x1a4/0x1a8 [ 12.924289] ---[ end trace 0000000000000000 ]--- ...
I'm not sure if this is now the expected behavior or not. There will be no backtraces, if I build a preemptible kernel, nor will there be backtraces if I revert this patch.
OTOH, the driver I'm using (drivers/i2c/busses/i2c-mt65xx.c) has no *_atomic(). So the warning is correct. There is also [1], which seems to be the same issue I'm facing.
-michael
[1] https://lore.kernel.org/linux-i2c/13271b9b-4132-46ef-abf8-2c311967bb46@mailb...