diff --git a/scst/include/scst.h b/scst/include/scst.h index 153b8a9e9..30d0e2d34 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -2081,7 +2081,7 @@ struct scst_cmd { struct scst_session *sess; /* corresponding session */ - atomic_t *cpu_cmd_counter; + bool counted; atomic_t cmd_ref; @@ -2571,7 +2571,7 @@ struct scst_mgmt_cmd { struct scst_session *sess; - atomic_t *cpu_cmd_counter; + bool counted; /* Mgmt cmd state, one of SCST_MCMD_STATE_* constants */ int state; diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 23a4f0acd..61eb4e789 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -5824,7 +5824,7 @@ struct scst_cmd *__scst_create_prepare_internal_cmd(const uint8_t *cdb, } scst_sess_get(res->sess); - res->cpu_cmd_counter = scst_get(); + scst_get_icmd(res); TRACE(TRACE_SCSI, "New internal cmd %p (op %s)", res, scst_get_opcode_name(res)); @@ -7534,10 +7534,8 @@ static void scst_destroy_cmd(struct scst_cmd *cmd) scst_sess_put(cmd->sess); - if (likely(cmd->cpu_cmd_counter)) { - scst_put(cmd->cpu_cmd_counter); - cmd->cpu_cmd_counter = NULL; - } + if (likely(cmd->counted)) + scst_put_cmd(cmd); EXTRACHECKS_BUG_ON(cmd->pre_alloced && cmd->internal); @@ -7777,10 +7775,8 @@ void scst_free_mgmt_cmd(struct scst_mgmt_cmd *mcmd) scst_sess_put(mcmd->sess); - if (mcmd->cpu_cmd_counter) { - scst_put(mcmd->cpu_cmd_counter); - mcmd->cpu_cmd_counter = NULL; - } + if (mcmd->counted) + scst_put_mcmd(mcmd); mempool_free(mcmd, scst_mgmt_mempool); diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index 6fa785d15..f19889273 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -112,8 +112,6 @@ struct kmem_cache *scst_cmd_cachep; unsigned long scst_trace_flag; #endif -unsigned long scst_flags; - #if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 9, 0) unsigned long scst_poll_ns = SCST_DEF_POLL_NS; #endif @@ -122,6 +120,9 @@ int scst_max_tasklet_cmd = SCST_DEF_MAX_TASKLET_CMD; struct scst_cmd_threads scst_main_cmd_threads; +static bool percpu_ref_killed; +struct percpu_ref scst_cmd_count; +struct percpu_ref scst_mcmd_count; struct scst_percpu_info scst_percpu_infos[NR_CPUS]; spinlock_t scst_mcmd_lock; @@ -796,13 +797,15 @@ static void __printf(2, 3) scst_to_syslog(void *arg, const char *fmt, ...) return; } +/* + * Number of SCST non-management commands, management commands and activities + * that are in progress. Must only be called if both scst_cmd_count and + * scst_mcmd_count are in atomic mode. + */ int scst_get_cmd_counter(void) { - int i, res = 0; - - for (i = 0; i < ARRAY_SIZE(scst_percpu_infos); i++) - res += atomic_read(&scst_percpu_infos[i].cpu_cmd_count); - return res; + return percpu_ref_read(&scst_cmd_count) + + percpu_ref_read(&scst_mcmd_count); } static int scst_susp_wait(unsigned long timeout) @@ -820,7 +823,7 @@ static int scst_susp_wait(unsigned long timeout) t = min(timeout, SCST_SUSP_WAIT_REPORT_TIMEOUT); res = wait_event_interruptible_timeout(scst_dev_cmd_waitQ, - (scst_get_cmd_counter() == 0), t); + percpu_ref_killed, t); if (res > 0) { res = 0; goto out; @@ -836,13 +839,13 @@ static int scst_susp_wait(unsigned long timeout) if (timeout != SCST_SUSPEND_TIMEOUT_UNLIMITED) { res = wait_event_interruptible_timeout(scst_dev_cmd_waitQ, - (scst_get_cmd_counter() == 0), timeout - t); + percpu_ref_killed, timeout - t); if (res == 0) res = -EBUSY; else if (res > 0) res = 0; } else { - wait_event(scst_dev_cmd_waitQ, scst_get_cmd_counter() == 0); + wait_event(scst_dev_cmd_waitQ, percpu_ref_killed); res = 0; } @@ -855,17 +858,18 @@ out: } /* - * scst_suspend_activity() - globally suspend any activity + * scst_suspend_activity() - globally suspend activity * * Description: - * Globally suspends any activity and doesn't return, until there are any - * active commands (state after SCST_CMD_STATE_INIT). Timeout parameter sets - * max time this function will wait for suspending or interrupted by a - * signal with the corresponding error status < 0. If timeout is - * SCST_SUSPEND_TIMEOUT_UNLIMITED, then it will wait virtually forever. - * On success returns 0. + * Globally suspends SCSI command and SCSI management command processing and + * waits until all active commands have finished (state after + * SCST_CMD_STATE_INIT). The timeout parameter defines the maximum time this + * function will wait until activity has been suspended. If this function is + * interrupted by a signal, it returns a negative value. If the timeout value + * is SCST_SUSPEND_TIMEOUT_UNLIMITED, then it will wait virtually forever. + * Returns 0 upon success. * - * New arriving commands stay in the suspended state until + * Newly arriving commands remain in the suspended state until * scst_resume_activity() is called. */ int scst_suspend_activity(unsigned long timeout) @@ -892,14 +896,9 @@ int scst_suspend_activity(unsigned long timeout) if (suspend_count > 1) goto out_up; - set_bit(SCST_FLAG_SUSPENDING, &scst_flags); - set_bit(SCST_FLAG_SUSPENDED, &scst_flags); - /* - * Assignment of SCST_FLAG_SUSPENDING and SCST_FLAG_SUSPENDED must be - * ordered with cpu_cmd_count in scst_get(). Otherwise, lockless logic - * of scst_get() users won't work. - */ - smp_mb__after_set_bit(); + /* Cause scst_get_cmd() to fail. */ + percpu_ref_killed = false; + percpu_ref_kill(&scst_cmd_count); /* * See comment in scst_user.c::dev_user_task_mgmt_fn() for more @@ -921,12 +920,13 @@ int scst_suspend_activity(unsigned long timeout) } res = scst_susp_wait(timeout); - if (res != 0) - goto out_clear; - clear_bit(SCST_FLAG_SUSPENDING, &scst_flags); - /* See comment about smp_mb() above */ - smp_mb__after_clear_bit(); + /* Cause scst_get_mcmd() to fail. */ + percpu_ref_killed = false; + percpu_ref_kill(&scst_mcmd_count); + + if (res != 0) + goto out_resume; if (scst_get_cmd_counter() != 0) TRACE_MGMT_DBG("Waiting for %d active commands finally to " @@ -964,11 +964,6 @@ out: TRACE_EXIT_RES(res); return res; -out_clear: - clear_bit(SCST_FLAG_SUSPENDING, &scst_flags); - /* See comment about smp_mb() above */ - smp_mb__after_clear_bit(); - out_resume: __scst_resume_activity(); EXTRACHECKS_BUG_ON(suspend_count != 0); @@ -994,7 +989,8 @@ static void __scst_resume_activity(void) if (suspend_count > 0) goto out; - clear_bit(SCST_FLAG_SUSPENDED, &scst_flags); + percpu_ref_resurrect(&scst_mcmd_count); + percpu_ref_resurrect(&scst_cmd_count); mutex_lock(&scst_cmd_threads_mutex); list_for_each_entry(l, &scst_cmd_threads_list, lists_list_entry) { @@ -1004,7 +1000,7 @@ static void __scst_resume_activity(void) /* * Wait until scst_init_thread() either is waiting or has reexamined - * scst_flags. + * scst_cmd_count. */ spin_lock_irq(&scst_init_lock); spin_unlock_irq(&scst_init_lock); @@ -2318,6 +2314,13 @@ static void __init scst_print_config(void) PRINT_INFO("%s", buf); } +static void scst_suspended(struct percpu_ref *ref) +{ + WARN_ON_ONCE(ref != &scst_cmd_count && ref != &scst_mcmd_count); + percpu_ref_killed = true; + wake_up_all(&scst_dev_cmd_waitQ); +} + static int __init init_scst(void) { int res, i; @@ -2526,8 +2529,17 @@ static int __init init_scst(void) if (res != 0) goto out_destroy_sgv_pool; + res = percpu_ref_init(&scst_cmd_count, scst_suspended, + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); + if (res != 0) + goto out_unreg_interface; + + res = percpu_ref_init(&scst_mcmd_count, scst_suspended, + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); + if (res != 0) + goto out_cmd_count; + for (i = 0; i < 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, @@ -2565,10 +2577,14 @@ out: out_thread_free: scst_stop_global_threads(); + percpu_ref_exit(&scst_mcmd_count); +out_cmd_count: + percpu_ref_exit(&scst_cmd_count); + +out_unreg_interface: scsi_unregister_interface(&scst_interface); - out_destroy_sgv_pool: scst_sgv_pools_deinit(); scst_tg_cleanup(); @@ -2651,6 +2667,9 @@ static void __exit exit_scst(void) scst_deinit_threads(&scst_main_cmd_threads); + percpu_ref_exit(&scst_mcmd_count); + percpu_ref_exit(&scst_cmd_count); + scsi_unregister_interface(&scst_interface); diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index 21061c7a1..9f3437c3b 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -74,20 +74,6 @@ extern unsigned long scst_trace_flag; #endif -/** - ** Bits for scst_flags - **/ - -/* - * Set if new commands initialization is being suspended for a while. - * Used to let TM commands execute while preparing the suspend, since - * RESET or ABORT could be necessary to free SCSI commands. - */ -#define SCST_FLAG_SUSPENDING 0 - -/* Set if new commands initialization is suspended for a while */ -#define SCST_FLAG_SUSPENDED 1 - extern spinlock_t scst_measure_latency_lock; extern atomic_t scst_measure_latency; void scst_update_latency_stats(struct scst_cmd *cmd, int new_state); @@ -184,7 +170,6 @@ extern struct kmem_cache *scst_tgt_cachep; extern struct kmem_cache *scst_tgtd_cachep; extern struct kmem_cache *scst_acgd_cachep; -extern unsigned long scst_flags; extern struct list_head scst_template_list; extern struct list_head scst_dev_list; extern struct list_head scst_dev_type_list; @@ -217,8 +202,9 @@ 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; +extern struct percpu_ref scst_cmd_count; +extern struct percpu_ref scst_mcmd_count; struct scst_percpu_info { - atomic_t cpu_cmd_count; spinlock_t tasklet_lock; struct list_head tasklet_cmd_list; struct tasklet_struct tasklet; @@ -671,51 +657,64 @@ void scst_ext_blocking_done(struct scst_device *dev); int scst_get_suspend_count(void); /* - * Increases global SCST ref counters which prevent from entering into suspended - * activities stage, so protects from any global management operations. + * Increase the global command count if it has not been 'killed'. Use this + * function to protect regular commands. */ -static inline atomic_t *scst_get(void) +static inline bool __must_check scst_get_cmd(struct scst_cmd *cmd) { - atomic_t *a; - - /* - * Avoid that a high I/O load prevents activity to be suspended. See - * also http://sourceforge.net/p/scst/mailman/message/34074831/. - */ - if (unlikely(test_bit(SCST_FLAG_SUSPENDING, &scst_flags))) - mdelay(100); - - /* - * 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. - */ - a = &scst_percpu_infos[raw_smp_processor_id()].cpu_cmd_count; - atomic_inc(a); - 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; + if (!percpu_ref_tryget_live(&scst_cmd_count)) + return false; + cmd->counted = true; + return true; } /* - * 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. + * Increase the global management command count if it is not zero. Use this + * function to protect management commands. */ -static inline void scst_put(atomic_t *a) +static inline bool __must_check scst_get_mcmd(struct scst_mgmt_cmd *mcmd) { - int f; + if (!percpu_ref_tryget_live(&scst_mcmd_count)) + return false; + mcmd->counted = true; + return true; +} - f = atomic_dec_and_test(a); - /* See comment about smp_mb() in scst_suspend_activity() */ - 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 cpu_cmd_count %p (new value %d)", - a, atomic_read(a)); +/* + * Increase the global command count. Use this function to protect internal + * commands. + */ +static inline void scst_get_icmd(struct scst_cmd *cmd) +{ + percpu_ref_get(&scst_cmd_count); + cmd->counted = true; +} + +/* Decrease the global SCST refcount which prevents suspending activity. */ +static inline void scst_put_cmd(struct scst_cmd *cmd) +{ + WARN_ON_ONCE(!cmd->counted); + cmd->counted = false; + percpu_ref_put(&scst_cmd_count); +} + +static inline void scst_put_mcmd(struct scst_mgmt_cmd *mcmd) +{ + WARN_ON_ONCE(!mcmd->counted); + mcmd->counted = false; + percpu_ref_put(&scst_mcmd_count); +} + +/* Whether or not activities are being suspended or have been suspended. */ +static inline bool scst_activity_suspended(void) +{ + return percpu_ref_is_dying(&scst_cmd_count); +} + +/* Returns true if and only if regular commands have already been suspended. */ +static inline bool scst_mcmd_suspended(void) +{ + return percpu_ref_is_dying(&scst_mcmd_count); } int scst_get_cmd_counter(void); diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 494f6163d..a3ef29bd1 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -574,7 +574,7 @@ static void __scst_rx_cmd(struct scst_cmd *cmd, struct scst_session *sess, { TRACE_ENTRY(); - WARN_ON_ONCE(cmd->cpu_cmd_counter); + WARN_ON_ONCE(cmd->counted); cmd->sess = sess; scst_sess_get(sess); @@ -4159,7 +4159,7 @@ struct scst_tgt_dev *scst_lookup_tgt_dev(struct scst_session *sess, u64 lun) * scst_translate_lun() - Translate @cmd->lun into a tgt_dev pointer. * @cmd: SCSI command for which to translate the LUN number. * - * Initialize the following @cmd members: cpu_cmd_counter, cmd_threads, + * Initialize the following @cmd members: counted, cmd_threads, * tgt_dev, cur_order_data, dev and devt. * * The caller must not hold any locks. May be called from IRQ context. The data @@ -4177,9 +4177,7 @@ static int scst_translate_lun(struct scst_cmd *cmd) TRACE_ENTRY(); - cmd->cpu_cmd_counter = scst_get(); - - if (likely(!test_bit(SCST_FLAG_SUSPENDED, &scst_flags))) { + if (likely(scst_get_cmd(cmd))) { TRACE_DBG("Finding tgt_dev for cmd %p (lun %lld)", cmd, (unsigned long long)cmd->lun); res = -1; @@ -4222,12 +4220,9 @@ static int scst_translate_lun(struct scst_cmd *cmd) cmd->sess->initiator_name, cmd->tgt->tgt_name); scst_event_queue_lun_not_found(cmd); } - scst_put(cmd->cpu_cmd_counter); - cmd->cpu_cmd_counter = NULL; + scst_put_cmd(cmd); } } else { - scst_put(cmd->cpu_cmd_counter); - cmd->cpu_cmd_counter = NULL; TRACE_MGMT_DBG("%s", "FLAG SUSPENDED set, skipping"); res = 1; } @@ -4436,7 +4431,8 @@ restart: * There is no need for read barrier here, because we don't care where * this check will be done. */ - susp = test_bit(SCST_FLAG_SUSPENDED, &scst_flags); + susp = scst_activity_suspended(); + if (scst_init_poll_cnt > 0) scst_init_poll_cnt--; @@ -4497,13 +4493,13 @@ restart: return; } -static inline int test_init_cmd_list(void) +/* Whether or not scst_init_thread() should stop waiting. */ +static inline bool test_init_cmd_list(void) { - int res = (!list_empty(&scst_init_cmd_list) && - !test_bit(SCST_FLAG_SUSPENDED, &scst_flags)) || - unlikely(kthread_should_stop()) || - (scst_init_poll_cnt > 0); - return res; + return (!list_empty(&scst_init_cmd_list) && + !scst_activity_suspended()) || + unlikely(kthread_should_stop()) || + (scst_init_poll_cnt > 0); } int scst_init_thread(void *arg) @@ -5024,9 +5020,8 @@ void scst_cmd_tasklet(long p) } /* - * Returns 0 on success, or > 0 if SCST_FLAG_SUSPENDED set and - * SCST_FLAG_SUSPENDING - not. No locks, protection is done by the - * suspended activity. + * Returns 0 on success, or > 0 upon failure. No locks, protection is done by + * suspending activity. */ static int scst_get_mgmt(struct scst_mgmt_cmd *mcmd) { @@ -5034,12 +5029,7 @@ static int scst_get_mgmt(struct scst_mgmt_cmd *mcmd) TRACE_ENTRY(); - mcmd->cpu_cmd_counter = scst_get(); - - if (unlikely(test_bit(SCST_FLAG_SUSPENDED, &scst_flags) && - !test_bit(SCST_FLAG_SUSPENDING, &scst_flags))) { - scst_put(mcmd->cpu_cmd_counter); - mcmd->cpu_cmd_counter = NULL; + if (unlikely(!scst_get_mcmd(mcmd))) { TRACE_MGMT_DBG("%s", "FLAG SUSPENDED set, skipping"); res = 1; goto out; @@ -5051,9 +5041,8 @@ out: } /* - * Returns 0 on success, < 0 if there is no device handler or - * > 0 if SCST_FLAG_SUSPENDED set and SCST_FLAG_SUSPENDING - not. - * No locks, protection is done by the suspended activity. + * Returns 0 on success, < 0 if there is no device handler or > 0 if activity + * has been suspended. No locks, protection is done by the suspended activity. */ static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) { @@ -5078,8 +5067,7 @@ static int scst_mgmt_translate_lun(struct scst_mgmt_cmd *mcmd) mcmd->mcmd_tgt_dev = tgt_dev; res = 0; } else { - scst_put(mcmd->cpu_cmd_counter); - mcmd->cpu_cmd_counter = NULL; + scst_put_mcmd(mcmd); res = -1; } @@ -5911,10 +5899,14 @@ static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd) res = scst_set_mcmd_next_state(mcmd); goto out; } - __scst_cmd_get(cmd); tgt_dev = cmd->tgt_dev; - if (tgt_dev != NULL) - mcmd->cpu_cmd_counter = scst_get(); + if (tgt_dev && !scst_get_mcmd(mcmd)) { + TRACE_MGMT_DBG("Suspended; skipping mcmd"); + spin_unlock_irq(&sess->sess_list_lock); + res = 1; + goto ret; + } + __scst_cmd_get(cmd); spin_unlock_irq(&sess->sess_list_lock); TRACE_DBG("Cmd to abort %p for tag %llu found (tgt_dev %p)", cmd, (unsigned long long)mcmd->tag, tgt_dev); @@ -5972,6 +5964,7 @@ static int scst_mgmt_cmd_init(struct scst_mgmt_cmd *mcmd) out: scst_event_queue_tm_fn_received(mcmd); +ret: TRACE_EXIT_RES(res); return res; } @@ -6851,9 +6844,7 @@ int scst_tm_thread(void *arg) rc = scst_process_mgmt_cmd(mcmd); spin_lock_irq(&scst_mcmd_lock); if (rc > 0) { - if (test_bit(SCST_FLAG_SUSPENDED, &scst_flags) && - !test_bit(SCST_FLAG_SUSPENDING, - &scst_flags)) { + if (scst_mcmd_suspended()) { TRACE_MGMT_DBG("Adding mgmt cmd %p to " "head of delayed mgmt cmd list", mcmd);