| Message ID | 20230918225848.66463-1-junxiao.bi@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: target: fix deadlock by recursive locking | expand |
On 9/18/23 5:58 PM, Junxiao Bi wrote: > The following call trace shown a deadlock issue due to recursive locking > of mutex "device_mutex" , first lock acquire in target_for_each_device() > and second in target_free_device(). > > PID: 148266 TASK: ffff8be21ffb5d00 CPU: 10 COMMAND: "iscsi_ttx" > #0 [ffffa2bfc9ec3b18] __schedule at ffffffffa8060e7f > #1 [ffffa2bfc9ec3ba0] schedule at ffffffffa8061224 > #2 [ffffa2bfc9ec3bb8] schedule_preempt_disabled at ffffffffa80615ee > #3 [ffffa2bfc9ec3bc8] __mutex_lock at ffffffffa8062fd7 > #4 [ffffa2bfc9ec3c40] __mutex_lock_slowpath at ffffffffa80631d3 > #5 [ffffa2bfc9ec3c50] mutex_lock at ffffffffa806320c > #6 [ffffa2bfc9ec3c68] target_free_device at ffffffffc0935998 [target_core_mod] > #7 [ffffa2bfc9ec3c90] target_core_dev_release at ffffffffc092f975 [target_core_mod] > #8 [ffffa2bfc9ec3ca0] config_item_put at ffffffffa79d250f > #9 [ffffa2bfc9ec3cd0] config_item_put at ffffffffa79d2583 > #10 [ffffa2bfc9ec3ce0] target_devices_idr_iter at ffffffffc0933f3a [target_core_mod] > #11 [ffffa2bfc9ec3d00] idr_for_each at ffffffffa803f6fc > #12 [ffffa2bfc9ec3d60] target_for_each_device at ffffffffc0935670 [target_core_mod] > #13 [ffffa2bfc9ec3d98] transport_deregister_session at ffffffffc0946408 [target_core_mod] > #14 [ffffa2bfc9ec3dc8] iscsit_close_session at ffffffffc09a44a6 [iscsi_target_mod] > #15 [ffffa2bfc9ec3df0] iscsit_close_connection at ffffffffc09a4a88 [iscsi_target_mod] > #16 [ffffa2bfc9ec3df8] finish_task_switch at ffffffffa76e5d07 > #17 [ffffa2bfc9ec3e78] iscsit_take_action_for_connection_exit at ffffffffc0991c23 [iscsi_target_mod] > #18 [ffffa2bfc9ec3ea0] iscsi_target_tx_thread at ffffffffc09a403b [iscsi_target_mod] > #19 [ffffa2bfc9ec3f08] kthread at ffffffffa76d8080 > #20 [ffffa2bfc9ec3f50] ret_from_fork at ffffffffa8200364 > > Fixes: 36d4cb460bcb ("scsi: target: Avoid that EXTENDED COPY commands trigger lock inversion") > Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> > --- > drivers/target/target_core_device.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c > index b7ac60f4a219..b6523d4b9259 100644 > --- a/drivers/target/target_core_device.c > +++ b/drivers/target/target_core_device.c > @@ -843,7 +843,6 @@ sector_t target_to_linux_sector(struct se_device *dev, sector_t lb) > EXPORT_SYMBOL(target_to_linux_sector); > > struct devices_idr_iter { > - struct config_item *prev_item; > int (*fn)(struct se_device *dev, void *data); > void *data; > }; > @@ -853,11 +852,9 @@ static int target_devices_idr_iter(int id, void *p, void *data) > { > struct devices_idr_iter *iter = data; > struct se_device *dev = p; > + struct config_item *item; > int ret; > > - config_item_put(iter->prev_item); I can't see why we did that (I had reviewed/signed-off on the patch that added the bug) and I don't see any reason why we would have needed it in the past. Looks like idr_for_each would have handled it ok back then as well. So patch looks ok to me. Reviewed-by: Mike Christie <michael.christie@oracle.com>
On Mon, 18 Sep 2023 15:58:48 -0700, Junxiao Bi wrote: > The following call trace shown a deadlock issue due to recursive locking > of mutex "device_mutex" , first lock acquire in target_for_each_device() > and second in target_free_device(). > > PID: 148266 TASK: ffff8be21ffb5d00 CPU: 10 COMMAND: "iscsi_ttx" > #0 [ffffa2bfc9ec3b18] __schedule at ffffffffa8060e7f > #1 [ffffa2bfc9ec3ba0] schedule at ffffffffa8061224 > #2 [ffffa2bfc9ec3bb8] schedule_preempt_disabled at ffffffffa80615ee > #3 [ffffa2bfc9ec3bc8] __mutex_lock at ffffffffa8062fd7 > #4 [ffffa2bfc9ec3c40] __mutex_lock_slowpath at ffffffffa80631d3 > #5 [ffffa2bfc9ec3c50] mutex_lock at ffffffffa806320c > #6 [ffffa2bfc9ec3c68] target_free_device at ffffffffc0935998 [target_core_mod] > #7 [ffffa2bfc9ec3c90] target_core_dev_release at ffffffffc092f975 [target_core_mod] > #8 [ffffa2bfc9ec3ca0] config_item_put at ffffffffa79d250f > #9 [ffffa2bfc9ec3cd0] config_item_put at ffffffffa79d2583 > #10 [ffffa2bfc9ec3ce0] target_devices_idr_iter at ffffffffc0933f3a [target_core_mod] > #11 [ffffa2bfc9ec3d00] idr_for_each at ffffffffa803f6fc > #12 [ffffa2bfc9ec3d60] target_for_each_device at ffffffffc0935670 [target_core_mod] > #13 [ffffa2bfc9ec3d98] transport_deregister_session at ffffffffc0946408 [target_core_mod] > #14 [ffffa2bfc9ec3dc8] iscsit_close_session at ffffffffc09a44a6 [iscsi_target_mod] > #15 [ffffa2bfc9ec3df0] iscsit_close_connection at ffffffffc09a4a88 [iscsi_target_mod] > #16 [ffffa2bfc9ec3df8] finish_task_switch at ffffffffa76e5d07 > #17 [ffffa2bfc9ec3e78] iscsit_take_action_for_connection_exit at ffffffffc0991c23 [iscsi_target_mod] > #18 [ffffa2bfc9ec3ea0] iscsi_target_tx_thread at ffffffffc09a403b [iscsi_target_mod] > #19 [ffffa2bfc9ec3f08] kthread at ffffffffa76d8080 > #20 [ffffa2bfc9ec3f50] ret_from_fork at ffffffffa8200364 > > [...] Applied to 6.6/scsi-fixes, thanks! [1/1] scsi: target: fix deadlock by recursive locking https://git.kernel.org/mkp/scsi/c/a154f5f643c6
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index b7ac60f4a219..b6523d4b9259 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -843,7 +843,6 @@ sector_t target_to_linux_sector(struct se_device *dev, sector_t lb) EXPORT_SYMBOL(target_to_linux_sector); struct devices_idr_iter { - struct config_item *prev_item; int (*fn)(struct se_device *dev, void *data); void *data; }; @@ -853,11 +852,9 @@ static int target_devices_idr_iter(int id, void *p, void *data) { struct devices_idr_iter *iter = data; struct se_device *dev = p; + struct config_item *item; int ret; - config_item_put(iter->prev_item); - iter->prev_item = NULL; - /* * We add the device early to the idr, so it can be used * by backend modules during configuration. We do not want @@ -867,12 +864,13 @@ static int target_devices_idr_iter(int id, void *p, void *data) if (!target_dev_configured(dev)) return 0; - iter->prev_item = config_item_get_unless_zero(&dev->dev_group.cg_item); - if (!iter->prev_item) + item = config_item_get_unless_zero(&dev->dev_group.cg_item); + if (!item) return 0; mutex_unlock(&device_mutex); ret = iter->fn(dev, iter->data); + config_item_put(item); mutex_lock(&device_mutex); return ret; @@ -895,7 +893,6 @@ int target_for_each_device(int (*fn)(struct se_device *dev, void *data), mutex_lock(&device_mutex); ret = idr_for_each(&devices_idr, target_devices_idr_iter, &iter); mutex_unlock(&device_mutex); - config_item_put(iter.prev_item); return ret; }
The following call trace shown a deadlock issue due to recursive locking of mutex "device_mutex" , first lock acquire in target_for_each_device() and second in target_free_device(). PID: 148266 TASK: ffff8be21ffb5d00 CPU: 10 COMMAND: "iscsi_ttx" #0 [ffffa2bfc9ec3b18] __schedule at ffffffffa8060e7f #1 [ffffa2bfc9ec3ba0] schedule at ffffffffa8061224 #2 [ffffa2bfc9ec3bb8] schedule_preempt_disabled at ffffffffa80615ee #3 [ffffa2bfc9ec3bc8] __mutex_lock at ffffffffa8062fd7 #4 [ffffa2bfc9ec3c40] __mutex_lock_slowpath at ffffffffa80631d3 #5 [ffffa2bfc9ec3c50] mutex_lock at ffffffffa806320c #6 [ffffa2bfc9ec3c68] target_free_device at ffffffffc0935998 [target_core_mod] #7 [ffffa2bfc9ec3c90] target_core_dev_release at ffffffffc092f975 [target_core_mod] #8 [ffffa2bfc9ec3ca0] config_item_put at ffffffffa79d250f #9 [ffffa2bfc9ec3cd0] config_item_put at ffffffffa79d2583 #10 [ffffa2bfc9ec3ce0] target_devices_idr_iter at ffffffffc0933f3a [target_core_mod] #11 [ffffa2bfc9ec3d00] idr_for_each at ffffffffa803f6fc #12 [ffffa2bfc9ec3d60] target_for_each_device at ffffffffc0935670 [target_core_mod] #13 [ffffa2bfc9ec3d98] transport_deregister_session at ffffffffc0946408 [target_core_mod] #14 [ffffa2bfc9ec3dc8] iscsit_close_session at ffffffffc09a44a6 [iscsi_target_mod] #15 [ffffa2bfc9ec3df0] iscsit_close_connection at ffffffffc09a4a88 [iscsi_target_mod] #16 [ffffa2bfc9ec3df8] finish_task_switch at ffffffffa76e5d07 #17 [ffffa2bfc9ec3e78] iscsit_take_action_for_connection_exit at ffffffffc0991c23 [iscsi_target_mod] #18 [ffffa2bfc9ec3ea0] iscsi_target_tx_thread at ffffffffc09a403b [iscsi_target_mod] #19 [ffffa2bfc9ec3f08] kthread at ffffffffa76d8080 #20 [ffffa2bfc9ec3f50] ret_from_fork at ffffffffa8200364 Fixes: 36d4cb460bcb ("scsi: target: Avoid that EXTENDED COPY commands trigger lock inversion") Signed-off-by: Junxiao Bi <junxiao.bi@oracle.com> --- drivers/target/target_core_device.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)