From cbb7de538ff4a38bee8f6ef7db3e1bc947d3e232 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 14 Jan 2011 21:05:25 +0000 Subject: [PATCH] More performance and scalability improvements git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@3222 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 7 ++--- scst/src/scst_lib.c | 17 +++++------- scst/src/scst_main.c | 61 +++++++++++++++----------------------------- scst/src/scst_priv.h | 52 +++++++++++++++++++++++++++---------- scst/src/scst_targ.c | 44 +++++++++++++++----------------- 5 files changed, 91 insertions(+), 90 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index e861bf8c1..924331421 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -1713,6 +1713,8 @@ struct scst_cmd { struct scst_session *sess; /* corresponding session */ + atomic_t *cpu_cmd_counter; + /* Cmd state, one of SCST_CMD_STATE_* constants */ int state; @@ -2066,6 +2068,8 @@ struct scst_mgmt_cmd { struct scst_session *sess; + atomic_t *cpu_cmd_counter; + /* Mgmt cmd state, one of SCST_MCMD_STATE_* constants */ int state; @@ -3891,9 +3895,6 @@ unsigned long scst_random(void); void scst_set_resp_data_len(struct scst_cmd *cmd, int resp_data_len); -void scst_get(void); -void scst_put(void); - void scst_cmd_get(struct scst_cmd *cmd); void scst_cmd_put(struct scst_cmd *cmd); diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 1f984e612..57b6437de 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -3710,7 +3710,7 @@ static struct scst_cmd *scst_create_prepare_internal_cmd( scst_sess_get(res->sess); if (res->tgt_dev != NULL) - __scst_get(); + res->cpu_cmd_counter = scst_get(); res->state = SCST_CMD_STATE_PARSE; @@ -4187,7 +4187,7 @@ static void scst_destroy_put_cmd(struct scst_cmd *cmd) * At this point tgt_dev can be dead, but the pointer remains non-NULL */ if (likely(cmd->tgt_dev != NULL)) - __scst_put(); + scst_put(cmd->cpu_cmd_counter); scst_destroy_cmd(cmd); return; @@ -4203,10 +4203,8 @@ void scst_free_cmd(struct scst_cmd *cmd) TRACE_DBG("Freeing cmd %p (tag %llu)", cmd, (long long unsigned int)cmd->tag); - if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) { - TRACE_MGMT_DBG("Freeing aborted cmd %p (scst_cmd_count %d)", - cmd, atomic_read(&scst_cmd_count)); - } + if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) + TRACE_MGMT_DBG("Freeing aborted cmd %p", cmd); sBUG_ON(cmd->unblock_dev); @@ -4373,7 +4371,7 @@ void scst_free_mgmt_cmd(struct scst_mgmt_cmd *mcmd) scst_sess_put(mcmd->sess); if (mcmd->mcmd_tgt_dev != NULL) - __scst_put(); + scst_put(mcmd->cpu_cmd_counter); mempool_free(mcmd, scst_mgmt_mempool); @@ -6772,9 +6770,8 @@ void scst_xmit_process_aborted_cmd(struct scst_cmd *cmd) { TRACE_ENTRY(); - TRACE_MGMT_DBG("Aborted cmd %p done (cmd_ref %d, " - "scst_cmd_count %d)", cmd, atomic_read(&cmd->cmd_ref), - atomic_read(&scst_cmd_count)); + TRACE_MGMT_DBG("Aborted cmd %p done (cmd_ref %d)", cmd, + atomic_read(&cmd->cmd_ref)); scst_done_cmd_mgmt(cmd); diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index 25e5da667..443b931cc 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -131,11 +131,10 @@ unsigned long scst_trace_flag; int scst_max_tasklet_cmd = SCST_DEF_MAX_TASKLET_CMD; unsigned long scst_flags; -atomic_t scst_cmd_count; struct scst_cmd_threads scst_main_cmd_threads; -struct scst_tasklet scst_tasklets[NR_CPUS]; +struct scst_percpu_info scst_percpu_infos[NR_CPUS]; spinlock_t scst_mcmd_lock; struct list_head scst_active_mgmt_cmd_list; @@ -662,6 +661,14 @@ again: } EXPORT_SYMBOL(scst_unregister_target); +int scst_get_cmd_counter(void) +{ + int i, res = 0; + for (i = 0; i < (int)ARRAY_SIZE(scst_percpu_infos); i++) + res += atomic_read(&scst_percpu_infos[i].cpu_cmd_count); + return res; +} + static int scst_susp_wait(bool interruptible) { int res = 0; @@ -670,7 +677,7 @@ static int scst_susp_wait(bool interruptible) if (interruptible) { res = wait_event_interruptible_timeout(scst_dev_cmd_waitQ, - (atomic_read(&scst_cmd_count) == 0), + (scst_get_cmd_counter() == 0), SCST_SUSPENDING_TIMEOUT); if (res <= 0) { __scst_resume_activity(); @@ -679,8 +686,7 @@ static int scst_susp_wait(bool interruptible) } else res = 0; } else - wait_event(scst_dev_cmd_waitQ, - atomic_read(&scst_cmd_count) == 0); + wait_event(scst_dev_cmd_waitQ, scst_get_cmd_counter() == 0); TRACE_MGMT_DBG("wait_event() returned %d", res); @@ -729,7 +735,7 @@ int scst_suspend_activity(bool interruptible) set_bit(SCST_FLAG_SUSPENDED, &scst_flags); /* * Assignment of SCST_FLAG_SUSPENDING and SCST_FLAG_SUSPENDED must be - * ordered with scst_cmd_count. Otherwise lockless logic in + * ordered with cpu_cmd_count in scst_get(). Otherwise lockless logic in * scst_translate_lun() and scst_mgmt_translate_lun() won't work. */ smp_mb__after_set_bit(); @@ -743,7 +749,7 @@ int scst_suspend_activity(bool interruptible) * implementation of scst_translate_lun().. ) */ - if (atomic_read(&scst_cmd_count) != 0) { + if (scst_get_cmd_counter() != 0) { PRINT_INFO("Waiting for %d active commands to complete... This " "might take few minutes for disks or few hours for " "tapes, if you use long executed commands, like " @@ -752,7 +758,7 @@ int scst_suspend_activity(bool interruptible) "responding to any commands, if might take virtually " "forever until the corresponding user space " "program recovers and starts responding or gets " - "killed.", atomic_read(&scst_cmd_count)); + "killed.", scst_get_cmd_counter()); rep = true; #if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 29) @@ -769,7 +775,7 @@ int scst_suspend_activity(bool interruptible) smp_mb__after_clear_bit(); TRACE_MGMT_DBG("Waiting for %d active commands finally to complete", - atomic_read(&scst_cmd_count)); + scst_get_cmd_counter()); res = scst_susp_wait(interruptible); if (res != 0) @@ -1982,31 +1988,6 @@ out_unlock: return res; } -/** - * scst_get() - increase global SCST ref counter - * - * Increases global SCST ref counter that prevents from entering into suspended - * activities stage, so protects from any global management operations. - */ -void scst_get(void) -{ - __scst_get(); -} -EXPORT_SYMBOL(scst_get); - -/** - * scst_put() - decrease global SCST ref counter - * - * Decreases global SCST ref counter that prevents from entering into suspended - * activities stage, so protects from any global management operations. On - * zero, if suspending activities is waiting, they will be suspended. - */ -void scst_put(void) -{ - __scst_put(); -} -EXPORT_SYMBOL(scst_put); - #ifndef CONFIG_SCST_PROC /** * scst_get_setup_id() - return SCST setup ID @@ -2181,7 +2162,6 @@ static int __init init_scst(void) #if defined(CONFIG_SCST_DEBUG) || defined(CONFIG_SCST_TRACING) scst_trace_flag = SCST_DEFAULT_LOG_FLAGS; #endif - atomic_set(&scst_cmd_count, 0); spin_lock_init(&scst_mcmd_lock); INIT_LIST_HEAD(&scst_active_mgmt_cmd_list); INIT_LIST_HEAD(&scst_delayed_mgmt_cmd_list); @@ -2328,12 +2308,13 @@ static int __init init_scst(void) goto out_destroy_sgv_pool; #endif - for (i = 0; i < (int)ARRAY_SIZE(scst_tasklets); i++) { - spin_lock_init(&scst_tasklets[i].tasklet_lock); - INIT_LIST_HEAD(&scst_tasklets[i].tasklet_cmd_list); - tasklet_init(&scst_tasklets[i].tasklet, + for (i = 0; i < (int)ARRAY_SIZE(scst_percpu_infos); i++) { + atomic_set(&scst_percpu_infos[i].cpu_cmd_count, 0); + spin_lock_init(&scst_percpu_infos[i].tasklet_lock); + INIT_LIST_HEAD(&scst_percpu_infos[i].tasklet_cmd_list); + tasklet_init(&scst_percpu_infos[i].tasklet, (void *)scst_cmd_tasklet, - (unsigned long)&scst_tasklets[i]); + (unsigned long)&scst_percpu_infos[i]); } TRACE_DBG("%d CPUs found, starting %d threads", scst_num_cpus, diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index a00dddbd7..58172740d 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -141,7 +141,6 @@ extern struct kmem_cache *scst_acgd_cachep; extern spinlock_t scst_main_lock; extern unsigned long scst_flags; -extern atomic_t scst_cmd_count; extern struct list_head scst_template_list; extern struct list_head scst_dev_list; extern struct list_head scst_dev_type_list; @@ -171,12 +170,13 @@ extern struct list_head scst_active_mgmt_cmd_list; extern struct list_head scst_delayed_mgmt_cmd_list; extern wait_queue_head_t scst_mgmt_cmd_list_waitQ; -struct scst_tasklet { +struct scst_percpu_info { + atomic_t cpu_cmd_count; spinlock_t tasklet_lock; struct list_head tasklet_cmd_list; struct tasklet_struct tasklet; -}; -extern struct scst_tasklet scst_tasklets[NR_CPUS]; +} ____cacheline_aligned_in_smp; +extern struct scst_percpu_info scst_percpu_infos[NR_CPUS]; extern wait_queue_head_t scst_mgmt_waitQ; extern spinlock_t scst_mgmt_lock; @@ -571,28 +571,54 @@ static inline void scst_check_unblock_dev(struct scst_cmd *cmd) return; } -static inline void __scst_get(void) +/* + * Increases global SCST ref counters which prevent from entering into suspended + * activities stage, so protects from any global management operations. + */ +static inline atomic_t *scst_get(void) { - atomic_inc(&scst_cmd_count); - TRACE_DBG("Incrementing scst_cmd_count(new value %d)", - atomic_read(&scst_cmd_count)); + atomic_t *a; + /* + * We don't mind if we because of preemption inc counter from another + * CPU as soon in the majority cases we will the correct one. So, let's + * have preempt_disable/enable only in the debug build to avoid warning. + */ +#ifdef CONFIG_DEBUG_PREEMPT + preempt_disable(); +#endif + a = &scst_percpu_infos[smp_processor_id()].cpu_cmd_count; + atomic_inc(a); +#ifdef CONFIG_DEBUG_PREEMPT + preempt_enable(); +#endif + TRACE_DBG("Incrementing cpu_cmd_count %p (new value %d)", + a, atomic_read(a)); /* See comment about smp_mb() in scst_suspend_activity() */ smp_mb__after_atomic_inc(); + + return a; } -static inline void __scst_put(void) +/* + * Decreases global SCST ref counters which prevent from entering into suspended + * activities stage, so protects from any global management operations. On + * all them zero, if suspending activities is waiting, it will be proceed. + */ +static inline void scst_put(atomic_t *a) { int f; - f = atomic_dec_and_test(&scst_cmd_count); + f = atomic_dec_and_test(a); /* See comment about smp_mb() in scst_suspend_activity() */ - if (f && unlikely(test_bit(SCST_FLAG_SUSPENDED, &scst_flags))) { + if (unlikely(test_bit(SCST_FLAG_SUSPENDED, &scst_flags)) && f) { TRACE_MGMT_DBG("%s", "Waking up scst_dev_cmd_waitQ"); wake_up_all(&scst_dev_cmd_waitQ); } - TRACE_DBG("Decrementing scst_cmd_count(new value %d)", - atomic_read(&scst_cmd_count)); + TRACE_DBG("Decrementing cpu_cmd_count %p (new value %d)", + a, atomic_read(a)); } +int scst_get_cmd_counter(void); + void scst_sched_session_free(struct scst_session *sess); static inline void scst_sess_get(struct scst_session *sess) diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 104159676..50e961e23 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -79,21 +79,21 @@ EXPORT_SYMBOL_GPL(scst_post_alloc_data_buf); static inline void scst_schedule_tasklet(struct scst_cmd *cmd) { - struct scst_tasklet *t = &scst_tasklets[smp_processor_id()]; + struct scst_percpu_info *i = &scst_percpu_infos[smp_processor_id()]; unsigned long flags; - if (atomic_read(&scst_cmd_count) <= scst_max_tasklet_cmd) { - spin_lock_irqsave(&t->tasklet_lock, flags); + if (atomic_read(&i->cpu_cmd_count) <= scst_max_tasklet_cmd) { + spin_lock_irqsave(&i->tasklet_lock, flags); TRACE_DBG("Adding cmd %p to tasklet %d cmd list", cmd, smp_processor_id()); - list_add_tail(&cmd->cmd_list_entry, &t->tasklet_cmd_list); - spin_unlock_irqrestore(&t->tasklet_lock, flags); + list_add_tail(&cmd->cmd_list_entry, &i->tasklet_cmd_list); + spin_unlock_irqrestore(&i->tasklet_lock, flags); - tasklet_schedule(&t->tasklet); + tasklet_schedule(&i->tasklet); } else { spin_lock_irqsave(&cmd->cmd_threads->cmd_list_lock, flags); TRACE_DBG("Too many tasklet commands (%d), adding cmd %p to " - "active cmd list", atomic_read(&scst_cmd_count), cmd); + "active cmd list", atomic_read(&i->cpu_cmd_count), cmd); list_add_tail(&cmd->cmd_list_entry, &cmd->cmd_threads->active_cmd_list); wake_up(&cmd->cmd_threads->cmd_list_waitQ); @@ -231,8 +231,7 @@ out_redirect: } else { unsigned long flags; spin_lock_irqsave(&scst_init_lock, flags); - TRACE_MGMT_DBG("Adding cmd %p to init cmd list (scst_cmd_count " - "%d)", cmd, atomic_read(&scst_cmd_count)); + TRACE_MGMT_DBG("Adding cmd %p to init cmd list)", cmd); list_add_tail(&cmd->cmd_list_entry, &scst_init_cmd_list); if (test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags)) scst_init_poll_cnt++; @@ -3602,9 +3601,8 @@ static int scst_finish_cmd(struct scst_cmd *cmd) smp_mb(); /* to sync with scst_abort_cmd() */ if (unlikely(test_bit(SCST_CMD_ABORTED, &cmd->cmd_flags))) { - TRACE_MGMT_DBG("Aborted cmd %p finished (cmd_ref %d, " - "scst_cmd_count %d)", cmd, atomic_read(&cmd->cmd_ref), - atomic_read(&scst_cmd_count)); + TRACE_MGMT_DBG("Aborted cmd %p finished (cmd_ref %d)", + cmd, atomic_read(&cmd->cmd_ref)); scst_finish_cmd_mgmt(cmd); } @@ -3754,8 +3752,7 @@ static int scst_translate_lun(struct scst_cmd *cmd) TRACE_ENTRY(); - /* See comment about smp_mb() in scst_suspend_activity() */ - __scst_get(); + cmd->cpu_cmd_counter = scst_get(); if (likely(!test_bit(SCST_FLAG_SUSPENDED, &scst_flags))) { struct list_head *head = @@ -3789,11 +3786,11 @@ static int scst_translate_lun(struct scst_cmd *cmd) "tgt_dev for LUN %lld not found, command to " "unexisting LU?", (long long unsigned int)cmd->lun); - __scst_put(); + scst_put(cmd->cpu_cmd_counter); } } else { TRACE_MGMT_DBG("%s", "FLAG SUSPENDED set, skipping"); - __scst_put(); + scst_put(cmd->cpu_cmd_counter); res = 1; } @@ -4299,13 +4296,13 @@ int scst_cmd_thread(void *arg) void scst_cmd_tasklet(long p) { - struct scst_tasklet *t = (struct scst_tasklet *)p; + struct scst_percpu_info *i = (struct scst_percpu_info *)p; TRACE_ENTRY(); - spin_lock_irq(&t->tasklet_lock); - scst_do_job_active(&t->tasklet_cmd_list, &t->tasklet_lock, true); - spin_unlock_irq(&t->tasklet_lock); + spin_lock_irq(&i->tasklet_lock); + scst_do_job_active(&i->tasklet_cmd_list, &i->tasklet_lock, true); + spin_unlock_irq(&i->tasklet_lock); TRACE_EXIT(); return; @@ -4327,13 +4324,12 @@ static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) TRACE_DBG("Finding tgt_dev for mgmt cmd %p (lun %lld)", mcmd, (long long unsigned int)mcmd->lun); - /* See comment about smp_mb() in scst_suspend_activity() */ - __scst_get(); + mcmd->cpu_cmd_counter = scst_get(); if (unlikely(test_bit(SCST_FLAG_SUSPENDED, &scst_flags) && !test_bit(SCST_FLAG_SUSPENDING, &scst_flags))) { TRACE_MGMT_DBG("%s", "FLAG SUSPENDED set, skipping"); - __scst_put(); + scst_put(mcmd->cpu_cmd_counter); res = 1; goto out; } @@ -4348,7 +4344,7 @@ static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) } } if (mcmd->mcmd_tgt_dev == NULL) - __scst_put(); + scst_put(mcmd->cpu_cmd_counter); out: TRACE_EXIT_HRES(res);