From 0bb6de9471bd324527b1b95c2abb823009fd5ed0 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 1 Mar 2017 15:31:59 +0000 Subject: [PATCH] scst_vdisk: Avoid that LUN refresh triggers a general protection fault Avoid that triggering LUN referesh concurrently with device deletion triggers the following: general protection fault: 0000 [#1] Workqueue: events vdev_inq_changed_fn [scst_vdisk] Call Trace: _raw_spin_lock_bh+0x2b/0x30 scst_cm_update_dev+0x87/0x190 [scst] scst_dev_inquiry_data_changed+0xfb/0x1b0 [scst] vdev_inq_changed_fn+0x60/0x120 [scst_vdisk] process_one_work+0x14d/0x410 worker_thread+0x66/0x460 kthread+0xdb/0x100 ret_from_fork+0x3f/0x70 Reported-by: Jinpu Wang Tested-by: Jinpu Wang git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7101 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 5 ++++- scst/src/dev_handlers/scst_user.c | 4 ++-- scst/src/dev_handlers/scst_vdisk.c | 20 +++++++++++++++++--- scst/src/scst_copy_mgr.c | 4 +--- scst/src/scst_main.c | 18 +++++++++++++++--- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 4229decef..d861d2906 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -3766,7 +3766,10 @@ static inline int scst_register_virtual_device(struct scst_dev_type *dev_handler return scst_register_virtual_device_node(dev_handler, dev_name, NUMA_NO_NODE); } -void scst_unregister_virtual_device(int id); +void scst_unregister_virtual_device(int id, + void (*on_free)(struct scst_device *dev, + void *arg), + void *arg); /* * Get/Set functions for tgt's sg_tablesize diff --git a/scst/src/dev_handlers/scst_user.c b/scst/src/dev_handlers/scst_user.c index 21a849372..591511664 100644 --- a/scst/src/dev_handlers/scst_user.c +++ b/scst/src/dev_handlers/scst_user.c @@ -3518,7 +3518,7 @@ out: return res; out_unreg_drv: - scst_unregister_virtual_device(dev->virt_id); + scst_unregister_virtual_device(dev->virt_id, NULL, NULL); out_unreg_handler: scst_unregister_virtual_dev_driver(&dev->devtype); @@ -3887,7 +3887,7 @@ static int dev_user_exit_dev(struct scst_user_dev *dev) wake_up(&cleanup_list_waitQ); - scst_unregister_virtual_device(dev->virt_id); + scst_unregister_virtual_device(dev->virt_id, NULL, NULL); scst_unregister_virtual_dev_driver(&dev->devtype); sgv_pool_flush(dev->pool_clust); diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index 870d40f10..ba4d7ba69 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -353,6 +353,7 @@ static enum compl_status_e vdisk_exec_write_same(struct vdisk_cmd_params *p); static int vdisk_fsync(loff_t loff, loff_t len, struct scst_device *dev, gfp_t gfp_flags, struct scst_cmd *cmd, bool async); +static void vdev_on_free(struct scst_device *dev, void *arg); #ifdef CONFIG_SCST_PROC static int vdisk_read_proc(struct seq_file *seq, struct scst_dev_type *dev_type); @@ -7875,8 +7876,6 @@ static inline int vdev_create(struct scst_dev_type *devt, static void vdev_destroy(struct scst_vdisk_dev *virt_dev) { - cancel_work_sync(&virt_dev->vdev_inq_changed_work); - #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 30) vdisk_free_bioset(virt_dev); #endif @@ -8403,12 +8402,27 @@ out: #endif /* CONFIG_SCST_PROC */ +static void vdev_on_free(struct scst_device *dev, void *arg) +{ + struct scst_vdisk_dev *virt_dev = arg; + + TRACE_DBG("%s(%s)", __func__, dev->virt_name ? : "(?)"); + + /* + * This call must happen after scst_unregister_virtual_device() + * has called scst_dev_sysfs_del() and before scst_free_device() + * starts deallocating *dev. + */ + cancel_work_sync(&virt_dev->vdev_inq_changed_work); +} + /* scst_vdisk_mutex supposed to be held */ static void vdev_del_device(struct scst_vdisk_dev *virt_dev) { TRACE_ENTRY(); - scst_unregister_virtual_device(virt_dev->virt_id); + scst_unregister_virtual_device(virt_dev->virt_id, vdev_on_free, + virt_dev); list_del(&virt_dev->vdev_list_entry); diff --git a/scst/src/scst_copy_mgr.c b/scst/src/scst_copy_mgr.c index 66a38eea4..a47fe237b 100644 --- a/scst/src/scst_copy_mgr.c +++ b/scst/src/scst_copy_mgr.c @@ -2345,7 +2345,7 @@ static void scst_cm_init_inq_finish(struct scst_cmd *cmd) /* cmd->dev can be NULL here! */ rc = scst_cm_err_check_retry(cmd, cmd->start_time, scst_cm_inq_retry_fn); - if (rc == SCST_CM_STATUS_RETRY) + if (rc == SCST_CM_STATUS_RETRY || !cmd->dev || !cmd->tgt_dev) goto out; spin_lock_bh(&dev->dev_lock); @@ -2544,8 +2544,6 @@ static unsigned int scst_cm_get_lun(const struct scst_device *dev) } } - WARN_ON_ONCE(res == SCST_MAX_LUN); - out: TRACE_EXIT_RES(res); return res; diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index bfae359bf..2743eb35a 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -1266,7 +1266,10 @@ static struct scst_device *__scst_lookup_device(struct scsi_device *scsidp) return NULL; } -static void scst_unregister_device(struct scsi_device *scsidp) +static void scst_unregister_device(struct scsi_device *scsidp, + void (*on_free)(struct scst_device *dev, + void* arg), + void *arg) { struct scst_device *dev; struct scst_acg_dev *acg_dev, *aa; @@ -1324,6 +1327,9 @@ static void scst_unregister_device(struct scsi_device *scsidp) scsidp->host->host_no, scsidp->channel, scsidp->id, (u64)scsidp->lun, scsidp->type); + if (on_free) + on_free(dev, arg); + scst_free_device(dev); out: @@ -1557,7 +1563,10 @@ EXPORT_SYMBOL_GPL(scst_register_virtual_device_node); * scst_unregister_virtual_device() - unegister a virtual device. * @id: the device's ID, returned by the registration function */ -void scst_unregister_virtual_device(int id) +void scst_unregister_virtual_device(int id, + void (*on_free)(struct scst_device *dev, + void *arg), + void *arg) { struct scst_device *d, *dev = NULL; struct scst_acg_dev *acg_dev, *aa; @@ -1604,6 +1613,9 @@ void scst_unregister_virtual_device(int id) PRINT_INFO("Detached from virtual device %s (id %d)", dev->virt_name, dev->virt_id); + if (on_free) + on_free(dev, arg); + scst_free_device(dev); out: @@ -2404,7 +2416,7 @@ static void scst_remove(struct device *cdev, struct class_interface *intf) if ((scsidp->host->hostt->name == NULL) || (strcmp(scsidp->host->hostt->name, SCST_LOCAL_NAME) != 0)) - scst_unregister_device(scsidp); + scst_unregister_device(scsidp, NULL, NULL); TRACE_EXIT(); return;