From 39287411e32aeeed342c7da8417975e97310cd4a Mon Sep 17 00:00:00 2001 From: Mr NeilBrown Date: Wed, 26 Feb 2020 09:51:21 +1100 Subject: [PATCH] LU-10467 libcfs: remove l_wait_event() and related macros. Now that all usage of l_wait_event() has been converted to standard wait_event* macros, we can remove l_wait_event and related macros. Also clean up a few comments that mentioned these removed macros. Test-Parameters: trivial Signed-off-by: Mr NeilBrown Change-Id: Ie5fdc21ac94520e52021aa870d351230d39b0bc7 Reviewed-on: https://review.whamcloud.com/37729 Tested-by: jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Shaun Tancheff Reviewed-by: Yang Sheng Reviewed-by: Oleg Drokin --- libcfs/include/libcfs/linux/linux-wait.h | 10 +- lustre/include/lustre_lib.h | 299 ------------------------------- lustre/osc/osc_cache.c | 2 +- lustre/osp/osp_precreate.c | 5 +- lustre/ptlrpc/client.c | 8 +- lustre/quota/qsd_handler.c | 5 +- 6 files changed, 19 insertions(+), 310 deletions(-) diff --git a/libcfs/include/libcfs/linux/linux-wait.h b/libcfs/include/libcfs/linux/linux-wait.h index d3fbf60..8fac3db 100644 --- a/libcfs/include/libcfs/linux/linux-wait.h +++ b/libcfs/include/libcfs/linux/linux-wait.h @@ -221,7 +221,13 @@ __out: __ret; \ \ if (condition) \ break; \ - /* See justification in __l_wait_event */ \ + /* We have to do this here because some signals */ \ + /* are not blockable - ie from strace(1). */ \ + /* In these cases we want to schedule_timeout() */ \ + /* again, because we don't want that to return */ \ + /* -EINTR when the RPC actually succeeded. */ \ + /* the recalc_sigpending() below will deliver the */ \ + /* signal properly. */ \ if (signal_pending(current)) { \ spin_lock_irqsave(¤t->sighand->siglock, \ flags); \ @@ -481,7 +487,7 @@ do { \ \ if (condition) \ break; \ - /* See justification in __l_wait_event */ \ + /* See justification in ___wait_event_idle */ \ if (signal_pending(current)) { \ spin_lock_irqsave(¤t->sighand->siglock, \ flags); \ diff --git a/lustre/include/lustre_lib.h b/lustre/include/lustre_lib.h index 6be8a40..fa6306e 100644 --- a/lustre/include/lustre_lib.h +++ b/lustre/include/lustre_lib.h @@ -56,7 +56,6 @@ struct ptlrpc_request; struct obd_export; struct lu_target; -struct l_wait_info; #include #include @@ -91,304 +90,6 @@ int do_set_info_async(struct obd_import *imp, void target_send_reply(struct ptlrpc_request *req, int rc, int fail_id); -/* - * l_wait_event is a flexible sleeping function, permitting simple caller - * configuration of interrupt and timeout sensitivity along with actions to - * be performed in the event of either exception. - * - * The first form of usage looks like this: - * - * struct l_wait_info lwi = LWI_TIMEOUT_INTR(timeout, timeout_handler, - * intr_handler, callback_data); - * rc = l_wait_event(waitq, condition, &lwi); - * - * l_wait_event() makes the current process wait on 'waitq' until 'condition' - * is TRUE or a "killable" signal (SIGTERM, SIKGILL, SIGINT) is pending. It - * returns 0 to signify 'condition' is TRUE, but if a signal wakes it before - * 'condition' becomes true, it optionally calls the specified 'intr_handler' - * if not NULL, and returns -EINTR. - * - * If a non-zero timeout is specified, signals are ignored until the timeout - * has expired. At this time, if 'timeout_handler' is not NULL it is called. - * If it returns FALSE l_wait_event() continues to wait as described above with - * signals enabled. Otherwise it returns -ETIMEDOUT. - * - * LWI_INTR(intr_handler, callback_data) is shorthand for - * LWI_TIMEOUT_INTR(0, NULL, intr_handler, callback_data) - * - * The second form of usage looks like this: - * - * struct l_wait_info lwi = LWI_TIMEOUT(timeout, timeout_handler); - * rc = l_wait_event(waitq, condition, &lwi); - * - * This form is the same as the first except that it COMPLETELY IGNORES - * SIGNALS. The caller must therefore beware that if 'timeout' is zero, or if - * 'timeout_handler' is not NULL and returns FALSE, then the ONLY thing that - * can unblock the current process is 'condition' becoming TRUE. - * - * Another form of usage is: - * struct l_wait_info lwi = LWI_TIMEOUT_INTERVAL(timeout, interval, - * timeout_handler); - * rc = l_wait_event(waitq, condition, &lwi); - * This is the same as previous case, but condition is checked once every - * 'interval' jiffies (if non-zero). - * - * Subtle synchronization point: this macro does *not* necessary takes - * wait-queue spin-lock before returning, and, hence, following idiom is safe - * ONLY when caller provides some external locking: - * - * Thread1 Thread2 - * - * l_wait_event(&obj->wq, ....); (1) - * - * wake_up(&obj->wq): (2) - * spin_lock(&q->lock); (2.1) - * __wake_up_common(q, ...); (2.2) - * spin_unlock(&q->lock, flags); (2.3) - * - * OBD_FREE_PTR(obj); (3) - * - * As l_wait_event() may "short-cut" execution and return without taking - * wait-queue spin-lock, some additional synchronization is necessary to - * guarantee that step (3) can begin only after (2.3) finishes. - * - * XXX nikita: some ptlrpc daemon threads have races of that sort. - * - */ -static inline int back_to_sleep(void *arg) -{ - return 0; -} - -#define LWI_ON_SIGNAL_NOOP ((void (*)(void *))(-1)) - -struct l_wait_info { - long lwi_timeout; - long lwi_interval; - int lwi_allow_intr; - int (*lwi_on_timeout)(void *); - void (*lwi_on_signal)(void *); - void *lwi_cb_data; -}; - -/* NB: LWI_TIMEOUT ignores signals completely */ -#define LWI_TIMEOUT(time, cb, data) \ -((struct l_wait_info) { \ - .lwi_timeout = time, \ - .lwi_on_timeout = cb, \ - .lwi_cb_data = data, \ - .lwi_interval = 0, \ - .lwi_allow_intr = 0 \ -}) - -#define LWI_TIMEOUT_INTERVAL(time, interval, cb, data) \ -((struct l_wait_info) { \ - .lwi_timeout = time, \ - .lwi_on_timeout = cb, \ - .lwi_cb_data = data, \ - .lwi_interval = interval, \ - .lwi_allow_intr = 0 \ -}) - -#define LWI_TIMEOUT_INTR(time, time_cb, sig_cb, data) \ -((struct l_wait_info) { \ - .lwi_timeout = time, \ - .lwi_on_timeout = time_cb, \ - .lwi_on_signal = sig_cb, \ - .lwi_cb_data = data, \ - .lwi_interval = 0, \ - .lwi_allow_intr = 0 \ -}) - -#define LWI_TIMEOUT_INTR_ALL(time, time_cb, sig_cb, data) \ -((struct l_wait_info) { \ - .lwi_timeout = time, \ - .lwi_on_timeout = time_cb, \ - .lwi_on_signal = sig_cb, \ - .lwi_cb_data = data, \ - .lwi_interval = 0, \ - .lwi_allow_intr = 1 \ -}) - -#define LWI_INTR(cb, data) LWI_TIMEOUT_INTR(0, NULL, cb, data) - -/** - * wait_queue_t of Linux (version < 2.6.34) is a FIFO list for exclusively - * waiting threads, which is not always desirable because all threads will - * be waken up again and again, even user only needs a few of them to be - * active most time. This is not good for performance because cache can - * be polluted by different threads. - * - * LIFO list can resolve this problem because we always wakeup the most - * recent active thread by default. - * - * NB: please don't call non-exclusive & exclusive wait on the same - * waitq if add_wait_queue_exclusive_head is used. - */ -#define add_wait_queue_exclusive_head(waitq, link) \ -{ \ - unsigned long flags; \ - \ - spin_lock_irqsave(&((waitq)->lock), flags); \ - __add_wait_queue_exclusive(waitq, link); \ - spin_unlock_irqrestore(&((waitq)->lock), flags); \ -} - -/* - * wait for @condition to become true, but no longer than timeout, specified - * by @info. - */ -#define __l_wait_event(wq, condition, info, ret, l_add_wait) \ -do { \ - wait_queue_entry_t __wait; \ - long __timeout = info->lwi_timeout; \ - sigset_t __blocked; \ - int __allow_intr = info->lwi_allow_intr; \ - \ - ret = 0; \ - if (condition) \ - break; \ - \ - init_waitqueue_entry(&__wait, current); \ - l_add_wait(&wq, &__wait); \ - \ - /* Block all signals (just the non-fatal ones if no timeout). */ \ - if (info->lwi_on_signal != NULL && (__timeout == 0 || __allow_intr)) \ - __blocked = cfs_block_sigsinv(LUSTRE_FATAL_SIGS); \ - else \ - __blocked = cfs_block_sigsinv(0); \ - \ - for (;;) { \ - set_current_state(TASK_INTERRUPTIBLE); \ - \ - /* To guarantee that the condition check will be done */ \ - /* after setting the thread state as TASK_INTERRUPTIBLE. */ \ - /* Otherwise, out-of-order execution may cause some race. */ \ - /* Consider the following real execution order: */ \ - \ - /* 1. Thread1 checks condition on CPU1, gets false. */ \ - /* 2. Thread2 sets condition on CPU2. */ \ - /* 3. Thread2 calls wake_up() on CPU2 to wake the threads */ \ - /* with state TASK_INTERRUPTIBLE | TASK_UNINTERRUPTIBLE. */ \ - /* But the Thread1's state is TASK_RUNNING at that time. */ \ - /* 4. Thread1 sets its state as TASK_INTERRUPTIBLE on CPU1, */ \ - /* then schedule. */ \ - \ - /* If the '__timeout' variable is zero, the Thread1 will */ \ - /* have no chance to check the condition again. */ \ - \ - /* Generally, the interval between out-of-ordered step1 and */ \ - /* step4 is very tiny, as to above step2 and step3 cannot */ \ - /* happen. On some degree, it can explain why we seldom hit */ \ - /* related trouble. But such race really exists, especially */ \ - /* consider that the step1 and step4 can be interruptible. */ \ - /* So add barrier to avoid Thread1 out-of-order execution. */ \ - smp_mb(); \ - \ - if (condition) \ - break; \ - \ - if (__timeout == 0) { \ - schedule(); \ - } else { \ - long interval = info->lwi_interval ? \ - min_t(long, info->lwi_interval,\ - __timeout) : __timeout; \ - long remaining = schedule_timeout(interval); \ - \ - __timeout -= interval - remaining; \ - if (__timeout == 0) { \ - if (info->lwi_on_timeout == NULL || \ - info->lwi_on_timeout(info->lwi_cb_data)) { \ - ret = -ETIMEDOUT; \ - break; \ - } \ - /* Take signals after the timeout expires. */ \ - if (info->lwi_on_signal != NULL) \ - (void)cfs_block_sigsinv(LUSTRE_FATAL_SIGS);\ - } \ - } \ - \ - if (condition) \ - break; \ - if (signal_pending(current)) { \ - unsigned long flags; \ - \ - if (info->lwi_on_signal != NULL && \ - (__timeout == 0 || __allow_intr)) { \ - if (info->lwi_on_signal != LWI_ON_SIGNAL_NOOP) \ - info->lwi_on_signal(info->lwi_cb_data);\ - ret = -EINTR; \ - break; \ - } \ - /* We have to do this here because some signals */ \ - /* are not blockable - ie from strace(1). */ \ - /* In these cases we want to schedule_timeout() */ \ - /* again, because we don't want that to return */ \ - /* -EINTR when the RPC actually succeeded. */ \ - /* the recalc_sigpending() below will deliver the */ \ - /* signal properly. */ \ - spin_lock_irqsave(¤t->sighand->siglock, flags); \ - clear_tsk_thread_flag(current, TIF_SIGPENDING); \ - spin_unlock_irqrestore(¤t->sighand->siglock, flags);\ - } \ - } \ - \ - cfs_restore_sigs(__blocked); \ - \ - set_current_state(TASK_RUNNING); \ - remove_wait_queue(&wq, &__wait); \ -} while (0) - - -#define l_wait_event(wq, condition, info) \ -({ \ - int __ret; \ - struct l_wait_info *__info = (info); \ - \ - __l_wait_event(wq, condition, __info, \ - __ret, add_wait_queue); \ - __ret; \ -}) - -#define l_wait_event_exclusive(wq, condition, info) \ -({ \ - int __ret; \ - struct l_wait_info *__info = (info); \ - \ - __l_wait_event(wq, condition, __info, \ - __ret, add_wait_queue_exclusive); \ - __ret; \ -}) - -#define l_wait_event_exclusive_head(wq, condition, info) \ -({ \ - int __ret; \ - struct l_wait_info *__info = (info); \ - \ - __l_wait_event(wq, condition, __info, \ - __ret, add_wait_queue_exclusive_head); \ - __ret; \ -}) - -#define l_wait_condition(wq, condition) \ -({ \ - struct l_wait_info lwi = { 0 }; \ - l_wait_event(wq, condition, &lwi); \ -}) - -#define l_wait_condition_exclusive(wq, condition) \ -({ \ - struct l_wait_info lwi = { 0 }; \ - l_wait_event_exclusive(wq, condition, &lwi); \ -}) - -#define l_wait_condition_exclusive_head(wq, condition) \ -({ \ - struct l_wait_info lwi = { 0 }; \ - l_wait_event_exclusive_head(wq, condition, &lwi); \ -}) - /** @} lib */ #endif /* _LUSTRE_LIB_H */ diff --git a/lustre/osc/osc_cache.c b/lustre/osc/osc_cache.c index 234e527..d3e2378 100644 --- a/lustre/osc/osc_cache.c +++ b/lustre/osc/osc_cache.c @@ -1616,7 +1616,7 @@ static int osc_enter_cache(const struct lu_env *env, struct client_obd *cli, spin_lock(&cli->cl_loi_list_lock); if (rc <= 0) { - /* l_wait_event is interrupted by signal or timed out */ + /* wait_event_idle_timeout timed out */ list_del_init(&ocw.ocw_entry); if (rc == 0) rc = -ETIMEDOUT; diff --git a/lustre/osp/osp_precreate.c b/lustre/osp/osp_precreate.c index c125948..7186b44 100644 --- a/lustre/osp/osp_precreate.c +++ b/lustre/osp/osp_precreate.c @@ -1466,8 +1466,9 @@ int osp_precreate_reserve(const struct lu_env *env, struct osp_device *d) synced = 1; } if (atomic_read(&d->opd_sync_rpcs_in_progress)) { - /* just wait till destroys are done */ - /* see l_wait_even() few lines below */ + /* just wait till destroys are done + * see wait_event_idle_timeout() below + */ } if (atomic_read(&d->opd_sync_changes) + atomic_read(&d->opd_sync_rpcs_in_progress) == 0) { diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c index ba4c079..b7be022 100644 --- a/lustre/ptlrpc/client.c +++ b/lustre/ptlrpc/client.c @@ -1910,7 +1910,7 @@ int ptlrpc_check_set(const struct lu_env *env, struct ptlrpc_request_set *set) } /* - * ptlrpc_set_wait->l_wait_event sets lwi_allow_intr + * ptlrpc_set_wait uses l_wait_event_abortable_timeout() * so it sets rq_intr regardless of individual rpc * timeouts. The synchronous IO waiting path sets * rq_intr irrespective of whether ptlrpcd @@ -2280,8 +2280,7 @@ int ptlrpc_expire_one_request(struct ptlrpc_request *req, int async_unlink) /** * Time out all uncompleted requests in request set pointed by \a data - * Callback used when waiting on sets with l_wait_event. - * Always returns 1. + * This is called when a wait times out. */ void ptlrpc_expired_set(struct ptlrpc_request_set *set) { @@ -2323,7 +2322,8 @@ void ptlrpc_expired_set(struct ptlrpc_request_set *set) /** * Interrupts (sets interrupted flag) all uncompleted requests in - * a set \a data. Callback for l_wait_event for interruptible waits. + * a set \a data. This is called when a wait_event is interrupted + * by a signal. */ static void ptlrpc_interrupted_set(struct ptlrpc_request_set *set) { diff --git a/lustre/quota/qsd_handler.c b/lustre/quota/qsd_handler.c index 2119271..f8011eb 100644 --- a/lustre/quota/qsd_handler.c +++ b/lustre/quota/qsd_handler.c @@ -623,7 +623,8 @@ static int qsd_acquire_remote(const struct lu_env *env, * \param space - is the amount of quota required for the operation * \param ret - is the return code (-EDQUOT, -EINPROGRESS, ...) * - * \retval true - exit from l_wait_event and real return value in \a ret + * \retval true - stop waiting in wait_event_idle_timeout, + * and real return value in \a ret * \retval false - continue waiting */ static bool qsd_acquire(const struct lu_env *env, struct lquota_entry *lqe, @@ -665,7 +666,7 @@ static bool qsd_acquire(const struct lu_env *env, struct lquota_entry *lqe, /* already a request in flight, continue waiting */ RETURN(false); *ret = rc; - RETURN(true); /* exit from l_wait_event */ + RETURN(true); } /** -- 1.8.3.1