Fixes recently reported problems:

- Possible crash on errors recovery in user space devices registration
 - Wrong handling of virtual devices with names looking similar to names of pass-through devices
 - Fix possible crash in scst_del_threads()

+ some other fixes and cleanups



git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@2017 d57e44dd-8a1f-0410-8b47-8ef2f437770f
This commit is contained in:
Vladislav Bolkhovitin
2010-08-28 12:38:52 +00:00
parent 725e090a94
commit 2a973f0df5
7 changed files with 123 additions and 252 deletions
+3 -1
View File
@@ -1524,9 +1524,11 @@ struct scst_cmd_threads {
* Those kernels don't support ref counting based IO context sharing
* between threads/processes, so need own ref counting.
*/
struct kref *io_context_kref;
struct kref io_context_kref;
#endif
bool io_context_ready;
int nr_threads; /* number of processing threads */
struct list_head threads_list; /* processing threads */
+4 -2
View File
@@ -2853,7 +2853,7 @@ static int dev_user_register_dev(struct file *file,
dev_desc->sgv_purge_interval);
if (dev->pool == NULL) {
res = -ENOMEM;
goto out_free_dev;
goto out_deinit_threads;
}
sgv_pool_set_allocator(dev->pool, dev_user_alloc_pages,
dev_user_free_sg_entries);
@@ -2968,7 +2968,9 @@ out_free:
out_free0:
sgv_pool_del(dev->pool);
out_free_dev:
out_deinit_threads:
scst_deinit_threads(&dev->udev_cmd_threads);
kfree(dev);
out_put:
+50 -98
View File
@@ -3005,7 +3005,7 @@ found:
t->acg_dev->acg->acg_io_grouping_type);
} else {
res = t;
if (res->active_cmd_threads->io_context == NULL) {
if ((volatile bool)!res->active_cmd_threads->io_context_ready) {
TRACE_MGMT_DBG("IO context for t %p not yet "
"initialized, waiting...", t);
msleep(100);
@@ -3081,25 +3081,6 @@ static int scst_ioc_keeper_thread(void *arg)
return 0;
}
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
static struct kref *scst_alloc_io_context_kref(void)
{
struct kref *io_context_kref;
io_context_kref = kmalloc(sizeof(*io_context_kref), GFP_KERNEL);
if (io_context_kref == NULL) {
PRINT_ERROR("Unable to alloc io_context_kref "
"(size %zd)", sizeof(*io_context_kref));
goto out;
}
kref_init(io_context_kref);
out:
return io_context_kref;
}
#endif
/* scst_mutex supposed to be held */
int scst_tgt_dev_setup_threads(struct scst_tgt_dev *tgt_dev)
{
@@ -3109,60 +3090,59 @@ int scst_tgt_dev_setup_threads(struct scst_tgt_dev *tgt_dev)
TRACE_ENTRY();
if (dev->threads_num <= 0) {
if (dev->threads_num < 0)
goto out;
if (dev->threads_num == 0) {
struct scst_tgt_dev *shared_io_tgt_dev;
tgt_dev->active_cmd_threads = &scst_main_cmd_threads;
if (dev->threads_num == 0) {
struct scst_tgt_dev *shared_io_tgt_dev;
shared_io_tgt_dev = scst_find_shared_io_tgt_dev(tgt_dev);
if (shared_io_tgt_dev != NULL) {
aic_keeper = shared_io_tgt_dev->aic_keeper;
kref_get(&aic_keeper->aic_keeper_kref);
shared_io_tgt_dev = scst_find_shared_io_tgt_dev(tgt_dev);
if (shared_io_tgt_dev != NULL) {
aic_keeper = shared_io_tgt_dev->aic_keeper;
kref_get(&aic_keeper->aic_keeper_kref);
TRACE_MGMT_DBG("Linking async io context %p "
"for shared tgt_dev %p (dev %s)",
aic_keeper->aic, tgt_dev,
tgt_dev->dev->virt_name);
} else {
/* Create new context */
aic_keeper = kzalloc(sizeof(*aic_keeper),
GFP_KERNEL);
if (aic_keeper == NULL) {
PRINT_ERROR("Unable to alloc aic_keeper "
"(size %zd)", sizeof(*aic_keeper));
res = -ENOMEM;
goto out;
}
kref_init(&aic_keeper->aic_keeper_kref);
init_waitqueue_head(&aic_keeper->aic_keeper_waitQ);
aic_keeper->aic_keeper_thr =
kthread_run(scst_ioc_keeper_thread,
aic_keeper, "aic_keeper");
if (IS_ERR(aic_keeper->aic_keeper_thr)) {
PRINT_ERROR("Error running ioc_keeper "
"thread (tgt_dev %p)", tgt_dev);
res = PTR_ERR(aic_keeper->aic_keeper_thr);
goto out_free_keeper;
}
wait_event(aic_keeper->aic_keeper_waitQ,
aic_keeper->aic != NULL);
TRACE_MGMT_DBG("Created async io context %p "
"for not shared tgt_dev %p (dev %s)",
aic_keeper->aic, tgt_dev,
tgt_dev->dev->virt_name);
TRACE_MGMT_DBG("Linking async io context %p "
"for shared tgt_dev %p (dev %s)",
aic_keeper->aic, tgt_dev,
tgt_dev->dev->virt_name);
} else {
/* Create new context */
aic_keeper = kzalloc(sizeof(*aic_keeper), GFP_KERNEL);
if (aic_keeper == NULL) {
PRINT_ERROR("Unable to alloc aic_keeper "
"(size %zd)", sizeof(*aic_keeper));
res = -ENOMEM;
goto out;
}
tgt_dev->async_io_context = aic_keeper->aic;
tgt_dev->aic_keeper = aic_keeper;
kref_init(&aic_keeper->aic_keeper_kref);
init_waitqueue_head(&aic_keeper->aic_keeper_waitQ);
aic_keeper->aic_keeper_thr =
kthread_run(scst_ioc_keeper_thread,
aic_keeper, "aic_keeper");
if (IS_ERR(aic_keeper->aic_keeper_thr)) {
PRINT_ERROR("Error running ioc_keeper "
"thread (tgt_dev %p)", tgt_dev);
res = PTR_ERR(aic_keeper->aic_keeper_thr);
goto out_free_keeper;
}
wait_event(aic_keeper->aic_keeper_waitQ,
aic_keeper->aic != NULL);
TRACE_MGMT_DBG("Created async io context %p "
"for not shared tgt_dev %p (dev %s)",
aic_keeper->aic, tgt_dev,
tgt_dev->dev->virt_name);
}
tgt_dev->async_io_context = aic_keeper->aic;
tgt_dev->aic_keeper = aic_keeper;
res = scst_add_threads(tgt_dev->active_cmd_threads, NULL, NULL,
tgt_dev->sess->tgt->tgtt->threads_num);
tgt_dev->sess->tgt->tgtt->threads_num);
goto out;
}
@@ -3184,23 +3164,7 @@ int scst_tgt_dev_setup_threads(struct scst_tgt_dev *tgt_dev)
/* It's ref counted via threads */
tgt_dev->active_cmd_threads->io_context =
shared_io_tgt_dev->active_cmd_threads->io_context;
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
tgt_dev->active_cmd_threads->io_context_kref =
shared_io_tgt_dev->active_cmd_threads->io_context_kref;
#endif
}
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
else {
struct kref *io_context_kref;
io_context_kref = scst_alloc_io_context_kref();
if (io_context_kref == NULL) {
res = -ENOMEM;
goto out;
}
tgt_dev->tgt_dev_cmd_threads.io_context_kref =
io_context_kref;
}
#endif
res = scst_add_threads(tgt_dev->active_cmd_threads, NULL,
tgt_dev,
@@ -3208,27 +3172,11 @@ int scst_tgt_dev_setup_threads(struct scst_tgt_dev *tgt_dev)
if (res != 0) {
/* Let's clear here, because no threads could be run */
tgt_dev->active_cmd_threads->io_context = NULL;
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
if (shared_io_tgt_dev == NULL) {
if (tgt_dev->active_cmd_threads->io_context_kref != NULL) {
kfree(tgt_dev->active_cmd_threads->io_context_kref);
tgt_dev->active_cmd_threads->io_context_kref = NULL;
}
}
#endif
}
break;
}
case SCST_THREADS_POOL_SHARED:
{
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
struct kref *io_context_kref = scst_alloc_io_context_kref();
if (io_context_kref == NULL) {
res = -ENOMEM;
goto out;
}
dev->dev_cmd_threads.io_context_kref = io_context_kref;
#endif
tgt_dev->active_cmd_threads = &dev->dev_cmd_threads;
res = scst_add_threads(tgt_dev->active_cmd_threads, dev, NULL,
@@ -3276,6 +3224,9 @@ void scst_tgt_dev_stop_threads(struct scst_tgt_dev *tgt_dev)
{
TRACE_ENTRY();
if (tgt_dev->dev->threads_num < 0)
goto out_deinit;
if (tgt_dev->active_cmd_threads == &scst_main_cmd_threads) {
/* Global async threads */
kref_put(&tgt_dev->aic_keeper->aic_keeper_kref,
@@ -3292,6 +3243,7 @@ void scst_tgt_dev_stop_threads(struct scst_tgt_dev *tgt_dev)
scst_deinit_threads(&tgt_dev->tgt_dev_cmd_threads);
} /* else no threads (not yet initialized, e.g.) */
out_deinit:
tm_dbg_deinit_tgt_dev(tgt_dev);
tgt_dev->active_cmd_threads = NULL;
+16 -15
View File
@@ -1452,7 +1452,7 @@ EXPORT_SYMBOL(scst_unregister_virtual_dev_driver);
int scst_add_threads(struct scst_cmd_threads *cmd_threads,
struct scst_device *dev, struct scst_tgt_dev *tgt_dev, int num)
{
int res, i;
int res = 0, i;
struct scst_cmd_thread_t *thr;
int n = 0, tgt_dev_num = 0;
@@ -1485,7 +1485,7 @@ int scst_add_threads(struct scst_cmd_threads *cmd_threads,
if (!thr) {
res = -ENOMEM;
PRINT_ERROR("Fail to allocate thr %d", res);
goto out_error;
goto out_wait;
}
if (dev != NULL) {
@@ -1506,7 +1506,7 @@ int scst_add_threads(struct scst_cmd_threads *cmd_threads,
res = PTR_ERR(thr->cmd_thread);
PRINT_ERROR("kthread_create() failed: %d", res);
kfree(thr);
goto out_error;
goto out_wait;
}
list_add(&thr->thread_list_entry, &cmd_threads->threads_list);
@@ -1518,27 +1518,25 @@ int scst_add_threads(struct scst_cmd_threads *cmd_threads,
wake_up_process(thr->cmd_thread);
}
out_wait:
if (cmd_threads != &scst_main_cmd_threads) {
/*
* Wait for io_context gets initialized to avoid possible races
* for it from the sharing it tgt_devs.
*/
while (cmd_threads->io_context == NULL) {
while ((volatile bool)!cmd_threads->io_context_ready) {
TRACE_DBG("Waiting for io_context for cmd_threads %p "
"initialized", cmd_threads);
msleep(50);
}
}
res = 0;
if (res != 0)
scst_del_threads(cmd_threads, i);
out:
TRACE_EXIT_RES(res);
return res;
out_error:
scst_del_threads(cmd_threads, i);
goto out;
}
/* scst_mutex supposed to be held */
@@ -1802,7 +1800,7 @@ void scst_deinit_threads(struct scst_cmd_threads *cmd_threads)
}
EXPORT_SYMBOL(scst_deinit_threads);
static void scst_stop_all_threads(void)
static void scst_stop_global_threads(void)
{
TRACE_ENTRY();
@@ -1824,7 +1822,7 @@ static void scst_stop_all_threads(void)
}
/* It does NOT stop ran threads on error! */
static int scst_start_all_threads(int num)
static int scst_start_global_threads(int num)
{
int res;
@@ -2096,7 +2094,7 @@ static int __init init_scst(void)
res = scst_lib_init();
if (res != 0)
goto out;
goto out_deinit_threads;
scst_num_cpus = num_online_cpus();
@@ -2236,7 +2234,7 @@ static int __init init_scst(void)
TRACE_DBG("%d CPUs found, starting %d threads", scst_num_cpus,
scst_threads);
res = scst_start_all_threads(scst_threads);
res = scst_start_global_threads(scst_threads);
if (res < 0)
goto out_thread_free;
@@ -2261,7 +2259,7 @@ out:
return res;
out_thread_free:
scst_stop_all_threads();
scst_stop_global_threads();
scsi_unregister_interface(&scst_interface);
@@ -2320,6 +2318,9 @@ out_destroy_mgmt_cache:
out_lib_exit:
scst_lib_exit();
out_deinit_threads:
scst_deinit_threads(&scst_main_cmd_threads);
goto out;
}
@@ -2333,7 +2334,7 @@ static void __exit exit_scst(void)
scst_proc_cleanup_module();
#endif
scst_stop_all_threads();
scst_stop_global_threads();
scst_deinit_threads(&scst_main_cmd_threads);
+16 -80
View File
@@ -1894,9 +1894,9 @@ static ssize_t scst_proc_groups_devices_write(struct file *file,
const char __user *buf,
size_t length, loff_t *off)
{
int res, action, virt = 0, rc, read_only = 0;
int res, action, rc, read_only = 0;
char *buffer, *p, *e = NULL;
unsigned int host, channel = 0, id = 0, lun = 0, virt_lun;
unsigned int virt_lun;
struct scst_acg *acg =
(struct scst_acg *)PDE(file->f_dentry->d_inode)->data;
struct scst_acg_dev *acg_dev = NULL, *acg_dev_tmp;
@@ -1977,48 +1977,19 @@ static ssize_t scst_proc_groups_devices_write(struct file *file,
while (isspace(*p) && *p != '\0')
p++;
e = p; /* save p */
host = simple_strtoul(p, &p, 0);
if (*p == ':') {
channel = simple_strtoul(p + 1, &p, 0);
id = simple_strtoul(p + 1, &p, 0);
lun = simple_strtoul(p + 1, &p, 0);
e = p;
} else {
virt++;
p = e; /* restore p */
while (!isspace(*e) && *e != '\0')
e++;
*e = 0;
}
while (!isspace(*e) && *e != '\0')
e++;
*e = 0;
list_for_each_entry(d, &scst_dev_list, dev_list_entry) {
if (virt) {
if (d->virt_id && !strcmp(d->virt_name, p)) {
dev = d;
TRACE_DBG("Virt device %p (%s) found",
dev, p);
break;
}
} else {
if (d->scsi_dev &&
d->scsi_dev->host->host_no == host &&
d->scsi_dev->channel == channel &&
d->scsi_dev->id == id &&
d->scsi_dev->lun == lun) {
dev = d;
TRACE_DBG("Dev %p (%d:%d:%d:%d) found",
dev, host, channel, id, lun);
break;
}
if (!strcmp(d->virt_name, p)) {
dev = d;
TRACE_DBG("Device %p (%s) found", dev, p);
break;
}
}
if (dev == NULL) {
if (virt) {
PRINT_ERROR("Virt device %s not found", p);
} else {
PRINT_ERROR("Device %d:%d:%d:%d not found",
host, channel, id, lun);
}
PRINT_ERROR("Device %s not found", p);
res = -EINVAL;
goto out_free_up;
}
@@ -2552,33 +2523,10 @@ static int scst_groups_devices_show(struct seq_file *seq, void *v)
"LUN", "Options");
list_for_each_entry(acg_dev, &acg->acg_dev_list, acg_dev_list_entry) {
if (acg_dev->dev->virt_id == 0) {
char conv[60];
int size = sizeof(conv);
memset(conv, 0, size);
size = snprintf(conv, size, "%d:%d:%d:",
acg_dev->dev->scsi_dev->host->host_no,
acg_dev->dev->scsi_dev->channel,
acg_dev->dev->scsi_dev->id);
seq_printf(seq, "%s", conv);
/*
* For some reason the third string argument always
* shown as NULL, so we have to split it on 2 calls.
*/
sprintf(conv, "%%-%dd%%-13d", 60 - size);
size += seq_printf(seq, conv,
acg_dev->dev->scsi_dev->lun,
acg_dev->lun);
seq_printf(seq, "%s\n",
acg_dev->rd_only ? "RO" : "");
} else {
seq_printf(seq, "%-60s%-13lld%s\n",
acg_dev->dev->virt_name,
(long long unsigned int)acg_dev->lun,
acg_dev->rd_only ? "RO" : "");
}
seq_printf(seq, "%-60s%-13lld%s\n",
acg_dev->dev->virt_name,
(long long unsigned int)acg_dev->lun,
acg_dev->rd_only ? "RO" : "");
}
mutex_unlock(&scst_mutex);
@@ -2674,20 +2622,8 @@ static int scst_tgt_info_show(struct seq_file *seq, void *v)
seq_printf(seq, "%-60s%s\n", "Device (host:ch:id:lun or name)",
"Device handler");
list_for_each_entry(dev, &scst_dev_list, dev_list_entry) {
if (dev->virt_id == 0) {
char conv[60];
int size = sizeof(conv);
size = snprintf(conv, size, "%d:%d:%d:",
dev->scsi_dev->host->host_no,
dev->scsi_dev->channel,
dev->scsi_dev->id);
seq_printf(seq, "%s", conv);
sprintf(conv, "%%-%dd%%s\n", 60 - size);
seq_printf(seq, conv, dev->scsi_dev->lun,
dev->handler ? dev->handler->name : "-");
} else
seq_printf(seq, "%-60s%s\n",
dev->virt_name, dev->handler->name);
seq_printf(seq, "%-60s%s\n",
dev->virt_name, dev->handler->name);
}
mutex_unlock(&scst_mutex);
+10 -39
View File
@@ -2552,9 +2552,9 @@ out_del:
static int __scst_process_luns_mgmt_store(char *buffer,
struct scst_tgt *tgt, struct scst_acg *acg, bool tgt_kobj)
{
int res, virt = 0, read_only = 0, action;
int res, read_only = 0, action;
char *p, *e = NULL;
unsigned int host, channel = 0, id = 0, lun = 0, virt_lun;
unsigned int virt_lun;
struct scst_acg_dev *acg_dev = NULL, *acg_dev_tmp;
struct scst_device *d, *dev = NULL;
@@ -2612,48 +2612,19 @@ static int __scst_process_luns_mgmt_store(char *buffer,
while (isspace(*p) && *p != '\0')
p++;
e = p; /* save p */
host = simple_strtoul(p, &p, 0);
if (*p == ':') {
channel = simple_strtoul(p + 1, &p, 0);
id = simple_strtoul(p + 1, &p, 0);
lun = simple_strtoul(p + 1, &p, 0);
e = p;
} else {
virt++;
p = e; /* restore p */
while (!isspace(*e) && *e != '\0')
e++;
*e = '\0';
}
while (!isspace(*e) && *e != '\0')
e++;
*e = '\0';
list_for_each_entry(d, &scst_dev_list, dev_list_entry) {
if (virt) {
if (d->virt_id && !strcmp(d->virt_name, p)) {
dev = d;
TRACE_DBG("Virt device %p (%s) found",
dev, p);
break;
}
} else {
if (d->scsi_dev &&
d->scsi_dev->host->host_no == host &&
d->scsi_dev->channel == channel &&
d->scsi_dev->id == id &&
d->scsi_dev->lun == lun) {
dev = d;
TRACE_DBG("Dev %p (%d:%d:%d:%d) found",
dev, host, channel, id, lun);
break;
}
if (!strcmp(d->virt_name, p)) {
dev = d;
TRACE_DBG("Device %p (%s) found", dev, p);
break;
}
}
if (dev == NULL) {
if (virt) {
PRINT_ERROR("Virt device '%s' not found", p);
} else {
PRINT_ERROR("Device %d:%d:%d:%d not found",
host, channel, id, lun);
}
PRINT_ERROR("Device '%s' not found", p);
res = -EINVAL;
goto out_unlock;
}
+24 -17
View File
@@ -4253,9 +4253,9 @@ static inline int test_cmd_threads(struct scst_cmd_threads *p_cmd_threads)
}
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
static void kref_free(struct kref *kref)
static void scst_io_context_kref_release(struct kref *kref)
{
kfree(kref);
/* Nothing to do */
}
#endif
@@ -4289,10 +4289,13 @@ int scst_cmd_thread(void *arg)
"(p_cmd_threads %p)",
p_cmd_threads->io_context,
p_cmd_threads);
/* It's ref counted via threads */
/*
* Put the extra reference. It isn't needed, because we
* ref counted via nr_threads below.
*/
put_io_context(p_cmd_threads->io_context);
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
/* p_cmd_threads->io_context_kref is already 1 */
kref_init(&p_cmd_threads->io_context_kref);
#endif
} else {
put_io_context(current->io_context);
@@ -4300,10 +4303,9 @@ int scst_cmd_thread(void *arg)
current->io_context = ioc_task_link(p_cmd_threads->io_context);
#else
current->io_context = p_cmd_threads->io_context;
kref_get(p_cmd_threads->io_context_kref);
TRACE_DBG("kref %p, new refcount %d",
p_cmd_threads->io_context_kref,
atomic_read(&p_cmd_threads->io_context_kref->refcount));
kref_get(&p_cmd_threads->io_context_kref);
TRACE_DBG("new refcount %d",
atomic_read(&p_cmd_threads->io_context_kref.refcount));
#endif
TRACE_MGMT_DBG("Linked IO context %p "
"(p_cmd_threads %p)", p_cmd_threads->io_context,
@@ -4313,6 +4315,8 @@ int scst_cmd_thread(void *arg)
mutex_unlock(&io_context_mutex);
p_cmd_threads->io_context_ready = true;
spin_lock_irq(&p_cmd_threads->cmd_list_lock);
while (!kthread_should_stop()) {
wait_queue_t wait;
@@ -4349,17 +4353,20 @@ int scst_cmd_thread(void *arg)
!list_empty(&p_cmd_threads->active_cmd_list));
if (p_cmd_threads != &scst_main_cmd_threads) {
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
TRACE_DBG("old refcount %d",
atomic_read(&p_cmd_threads->io_context_kref.refcount));
if (!kref_put(&p_cmd_threads->io_context_kref,
scst_io_context_kref_release)) {
/*
* Prevent io_context from being destroyed, we still
* need it.
*/
current->io_context = NULL;
}
#endif
if (p_cmd_threads->nr_threads == 1)
p_cmd_threads->io_context = NULL;
#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 25)
TRACE_DBG("kref %p, old refcount %d",
p_cmd_threads->io_context_kref,
atomic_read(&p_cmd_threads->io_context_kref->refcount));
if (kref_put(p_cmd_threads->io_context_kref, kref_free))
p_cmd_threads->io_context_kref = NULL;
else
current->io_context = NULL;
#endif
}
PRINT_INFO("Processing thread %s (PID %d) finished", current->comm,