Menu
▾
▴
[Scst-svn] [master] scst: Confirm percpu refs has scheduled and switched to atomic
|
From: Gleb C. <lna...@ya...> - 2023-06-28 16:32:18
|
Commit: 6a92549 GitHub URL: https://github.com/SCST-project/scst/commit/6a925490fd5447033aabb847ab3418a20aba4266 Author: Gleb Chesnokov Date: 2023-06-28T19:31:45+03:00 Log Message: ----------- scst: Confirm percpu refs has scheduled and switched to atomic This patch replaces percpu_ref_kill() with percpu_ref_kill_and_confirm() to guarantee safe usage of references in atomic mode immediately afterwards. This change ensures accurate checking of active commands following the initial reference killing. Reported-by: Lev Vainblat <le...@za...> Modified Paths: -------------- .github/workflows/checkpatch_pull.yml | 1 + .github/workflows/checkpatch_push.yml | 1 + scst/include/backport.h | 10 +++- scst/src/scst_main.c | 42 ++++++++++----- scst/src/scst_priv.h | 1 - 5 files changed, 40 insertions(+), 15 deletions(-) =================================================================== diff --git a/.github/workflows/checkpatch_pull.yml b/.github/workflows/checkpatch_pull.yml index c231e64..dfee81f 100644 --- a/.github/workflows/checkpatch_pull.yml +++ b/.github/workflows/checkpatch_pull.yml @@ -30,6 +30,7 @@ jobs: UNKNOWN_COMMIT_ID NO_AUTHOR_SIGN_OFF COMMIT_LOG_USE_LINK + BAD_REPORTED_BY_LINK FILE_PATH_CHANGES SPDX_LICENSE_TAG LINUX_VERSION_CODE diff --git a/.github/workflows/checkpatch_push.yml b/.github/workflows/checkpatch_push.yml index c5071f4..e517ccc 100644 --- a/.github/workflows/checkpatch_push.yml +++ b/.github/workflows/checkpatch_push.yml @@ -35,6 +35,7 @@ jobs: UNKNOWN_COMMIT_ID NO_AUTHOR_SIGN_OFF COMMIT_LOG_USE_LINK + BAD_REPORTED_BY_LINK FILE_PATH_CHANGES SPDX_LICENSE_TAG LINUX_VERSION_CODE diff --git a/scst/include/backport.h b/scst/include/backport.h index 6d0fd87..d8e873a 100644 --- a/scst/include/backport.h +++ b/scst/include/backport.h @@ -1098,13 +1098,21 @@ static inline void percpu_ref_put(struct percpu_ref *ref) ref->release(ref); } -static inline void percpu_ref_kill(struct percpu_ref *ref) +static inline void +percpu_ref_kill_and_confirm(struct percpu_ref *ref, percpu_ref_func_t *confirm_kill) { WARN_ON_ONCE(ref->dead); ref->dead = true; + if (confirm_kill) + confirm_kill(ref); percpu_ref_put(ref); } +static inline void percpu_ref_kill(struct percpu_ref *ref) +{ + percpu_ref_kill_and_confirm(ref, NULL); +} + static inline void percpu_ref_resurrect(struct percpu_ref *ref) { WARN_ON_ONCE(!ref->dead); diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index 11d26d3..3462a37 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -129,7 +129,8 @@ spinlock_t scst_mgmt_lock; struct list_head scst_sess_init_list; struct list_head scst_sess_shut_list; -wait_queue_head_t scst_dev_cmd_waitQ; +static wait_queue_head_t scst_dev_cmd_waitQ; +static struct completion scst_confirm_done; static struct mutex scst_cmd_threads_mutex; /* protected by scst_cmd_threads_mutex */ @@ -792,7 +793,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, - percpu_ref_killed, t); + percpu_ref_killed, t); if (res > 0) { res = 0; goto out; @@ -800,15 +801,16 @@ static int scst_susp_wait(unsigned long timeout) goto out; if (res == 0) { - PRINT_INFO("%d active commands to still not completed. See " - "README for possible reasons.", scst_get_cmd_counter()); + PRINT_INFO( + "%d active commands to still not completed. See README for possible reasons.", + scst_get_cmd_counter()); scst_trace_cmds(scst_to_syslog, &hp); scst_trace_mcmds(scst_to_syslog, &hp); } if (timeout != SCST_SUSPEND_TIMEOUT_UNLIMITED) { res = wait_event_interruptible_timeout(scst_dev_cmd_waitQ, - percpu_ref_killed, timeout - t); + percpu_ref_killed, timeout - t); if (res == 0) res = -EBUSY; else if (res > 0) @@ -826,6 +828,11 @@ out: #undef SCST_SUSP_WAIT_REPORT_TIMEOUT } +static void scst_suspend_counter_confirm(struct percpu_ref *ref) +{ + complete(&scst_confirm_done); +} + /* * scst_suspend_activity() - globally suspend activity * @@ -864,8 +871,12 @@ int scst_suspend_activity(unsigned long timeout) goto out_up; /* Cause scst_get_cmd() to fail. */ + init_completion(&scst_confirm_done); + percpu_ref_killed = false; - percpu_ref_kill(&scst_cmd_count); + percpu_ref_kill_and_confirm(&scst_cmd_count, scst_suspend_counter_confirm); + + wait_for_completion(&scst_confirm_done); /* * See comment in scst_user.c::dev_user_task_mgmt_fn() for more @@ -878,7 +889,7 @@ int scst_suspend_activity(unsigned long timeout) if (scst_get_cmd_counter() != 0) { PRINT_INFO("Waiting for %d active commands to complete...", - scst_get_cmd_counter()); + scst_get_cmd_counter()); rep = true; lock_contended(&scst_suspend_dep_map, _RET_IP_); @@ -887,15 +898,19 @@ int scst_suspend_activity(unsigned long timeout) res = scst_susp_wait(timeout); /* Cause scst_get_mcmd() to fail. */ + init_completion(&scst_confirm_done); + percpu_ref_killed = false; - percpu_ref_kill(&scst_mcmd_count); + percpu_ref_kill_and_confirm(&scst_mcmd_count, scst_suspend_counter_confirm); + + wait_for_completion(&scst_confirm_done); if (res != 0) goto out_resume; if (scst_get_cmd_counter() != 0) - TRACE_MGMT_DBG("Waiting for %d active commands finally to " - "complete", scst_get_cmd_counter()); + TRACE_MGMT_DBG("Waiting for %d active commands finally to complete", + scst_get_cmd_counter()); if (timeout != SCST_SUSPEND_TIMEOUT_UNLIMITED) { wait_time = jiffies - cur_time; @@ -913,7 +928,7 @@ int scst_suspend_activity(unsigned long timeout) goto out_resume; if (rep) - PRINT_INFO("%s", "All active commands completed"); + PRINT_INFO("All active commands completed"); out_up: mutex_unlock(&scst_suspend_mutex); @@ -973,8 +988,9 @@ static void __scst_resume_activity(void) spin_lock_irq(&scst_mcmd_lock); list_for_each_entry(m, &scst_delayed_mgmt_cmd_list, mgmt_cmd_list_entry) { - TRACE_MGMT_DBG("Moving delayed mgmt cmd %p to head of active " - "mgmt cmd list", m); + TRACE_MGMT_DBG( + "Moving delayed mgmt cmd %p to head of active mgmt cmd list", + m); } list_splice_init(&scst_delayed_mgmt_cmd_list, &scst_active_mgmt_cmd_list); diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index c9c0d3f..4927630 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -172,7 +172,6 @@ extern struct list_head scst_template_list; extern struct list_head scst_dev_list; extern struct list_head scst_dev_type_list; extern struct list_head scst_virtual_dev_type_list; -extern wait_queue_head_t scst_dev_cmd_waitQ; extern const struct scst_cl_ops scst_no_dlm_cl_ops; extern const struct scst_cl_ops scst_dlm_cl_ops; |