Whamcloud - gitweb
LU-10467 libcfs: remove l_wait_event() and related macros. 29/37729/4
authorMr NeilBrown <neilb@suse.de>
Tue, 25 Feb 2020 22:51:21 +0000 (09:51 +1100)
committerOleg Drokin <green@whamcloud.com>
Wed, 11 Mar 2020 02:10:16 +0000 (02:10 +0000)
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 <neilb@suse.de>
Change-Id: Ie5fdc21ac94520e52021aa870d351230d39b0bc7
Reviewed-on: https://review.whamcloud.com/37729
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
libcfs/include/libcfs/linux/linux-wait.h
lustre/include/lustre_lib.h
lustre/osc/osc_cache.c
lustre/osp/osp_precreate.c
lustre/ptlrpc/client.c
lustre/quota/qsd_handler.c

index d3fbf60..8fac3db 100644 (file)
@@ -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(&current->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(&current->sighand->siglock,   \
                                          flags);                       \
index 6be8a40..fa6306e 100644 (file)
@@ -56,7 +56,6 @@
 struct ptlrpc_request;
 struct obd_export;
 struct lu_target;
-struct l_wait_info;
 #include <lustre_ha.h>
 #include <lustre_net.h>
 
@@ -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(&current->sighand->siglock, flags);  \
-                       clear_tsk_thread_flag(current, TIF_SIGPENDING);        \
-                       spin_unlock_irqrestore(&current->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 */
index 234e527..d3e2378 100644 (file)
@@ -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;
index c125948..7186b44 100644 (file)
@@ -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) {
index ba4c079..b7be022 100644 (file)
@@ -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)
 {
index 2119271..f8011eb 100644 (file)
@@ -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);
 }
 
 /**