From owner-svn-src-all@freebsd.org Fri Nov 1 22:49:45 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AFDFF1A585C; Fri, 1 Nov 2019 22:49:45 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 474cp14GSvz3MmK; Fri, 1 Nov 2019 22:49:45 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 75C861A11F; Fri, 1 Nov 2019 22:49:45 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id xA1MnjRh089169; Fri, 1 Nov 2019 22:49:45 GMT (envelope-from mav@FreeBSD.org) Received: (from mav@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id xA1Mnj0A089165; Fri, 1 Nov 2019 22:49:45 GMT (envelope-from mav@FreeBSD.org) Message-Id: <201911012249.xA1Mnj0A089165@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mav set sender to mav@FreeBSD.org using -f From: Alexander Motin Date: Fri, 1 Nov 2019 22:49:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r354241 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: mav X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 354241 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Nov 2019 22:49:45 -0000 Author: mav Date: Fri Nov 1 22:49:44 2019 New Revision: 354241 URL: https://svnweb.freebsd.org/changeset/base/354241 Log: Some more taskqueue optimizations. - Optimize enqueue for two task priority values by adding new tq_hint field, pointing to the last task inserted into the middle of the list. In case of more then two priority values it should halve average search. - Move tq_active insert/remove out of the taskqueue_run_locked loop. Instead of dirtying few shared cache lines per task introduce different mechanism to drain active tasks, based on task sequence number counter, that uses only cache lines already present in cache. Since the new mechanism does not need ordering, switch tq_active from TAILQ to LIST. - Move static and dynamic struct taskqueue fields into different cache lines. Move lock into its own cache line, so that heavy lock spinning by multiple waiting threads would not affect the running thread. - While there, correct some TQ_SLEEP() wait messages. This change fixes certain ZFS write workloads, causing huge congestion on taskqueue lock. Those workloads combine some large block writes to saturate the pool and trigger allocation throttling, which uses higher priority tasks to requeue the delayed I/Os, with many small blocks to generate deep queue of small tasks for taskqueue to sort. MFC after: 1 week Sponsored by: iXsystems, Inc. Modified: head/sys/kern/subr_gtaskqueue.c head/sys/kern/subr_taskqueue.c Modified: head/sys/kern/subr_gtaskqueue.c ============================================================================== --- head/sys/kern/subr_gtaskqueue.c Fri Nov 1 21:26:43 2019 (r354240) +++ head/sys/kern/subr_gtaskqueue.c Fri Nov 1 22:49:44 2019 (r354241) @@ -57,26 +57,26 @@ TASKQGROUP_DEFINE(softirq, mp_ncpus, 1); TASKQGROUP_DEFINE(config, 1, 1); struct gtaskqueue_busy { - struct gtask *tb_running; - TAILQ_ENTRY(gtaskqueue_busy) tb_link; + struct gtask *tb_running; + u_int tb_seq; + LIST_ENTRY(gtaskqueue_busy) tb_link; }; -static struct gtask * const TB_DRAIN_WAITER = (struct gtask *)0x1; - typedef void (*gtaskqueue_enqueue_fn)(void *context); struct gtaskqueue { STAILQ_HEAD(, gtask) tq_queue; + LIST_HEAD(, gtaskqueue_busy) tq_active; + u_int tq_seq; + int tq_callouts; + struct mtx_padalign tq_mutex; gtaskqueue_enqueue_fn tq_enqueue; void *tq_context; char *tq_name; - TAILQ_HEAD(, gtaskqueue_busy) tq_active; - struct mtx tq_mutex; struct thread **tq_threads; int tq_tcount; int tq_spin; int tq_flags; - int tq_callouts; taskqueue_callback_fn tq_callbacks[TASKQUEUE_NUM_CALLBACKS]; void *tq_cb_contexts[TASKQUEUE_NUM_CALLBACKS]; }; @@ -115,12 +115,11 @@ gtask_dump(struct gtask *gtask) #endif static __inline int -TQ_SLEEP(struct gtaskqueue *tq, void *p, struct mtx *m, int pri, const char *wm, - int t) +TQ_SLEEP(struct gtaskqueue *tq, void *p, const char *wm) { if (tq->tq_spin) - return (msleep_spin(p, m, wm, t)); - return (msleep(p, m, pri, wm, t)); + return (msleep_spin(p, (struct mtx *)&tq->tq_mutex, wm, 0)); + return (msleep(p, &tq->tq_mutex, 0, wm, 0)); } static struct gtaskqueue * @@ -144,7 +143,7 @@ _gtaskqueue_create(const char *name, int mflags, } STAILQ_INIT(&queue->tq_queue); - TAILQ_INIT(&queue->tq_active); + LIST_INIT(&queue->tq_active); queue->tq_enqueue = enqueue; queue->tq_context = context; queue->tq_name = tq_name; @@ -167,7 +166,7 @@ gtaskqueue_terminate(struct thread **pp, struct gtaskq while (tq->tq_tcount > 0 || tq->tq_callouts > 0) { wakeup(tq); - TQ_SLEEP(tq, pp, &tq->tq_mutex, PWAIT, "taskqueue_destroy", 0); + TQ_SLEEP(tq, pp, "gtq_destroy"); } } @@ -178,7 +177,7 @@ gtaskqueue_free(struct gtaskqueue *queue) TQ_LOCK(queue); queue->tq_flags &= ~TQ_FLAGS_ACTIVE; gtaskqueue_terminate(queue->tq_threads, queue); - KASSERT(TAILQ_EMPTY(&queue->tq_active), ("Tasks still running?")); + KASSERT(LIST_EMPTY(&queue->tq_active), ("Tasks still running?")); KASSERT(queue->tq_callouts == 0, ("Armed timeout tasks")); mtx_destroy(&queue->tq_mutex); free(queue->tq_threads, M_GTASKQUEUE); @@ -285,7 +284,7 @@ gtaskqueue_drain_tq_queue(struct gtaskqueue *queue) * have completed or are currently executing. */ while (t_barrier.ta_flags & TASK_ENQUEUED) - TQ_SLEEP(queue, &t_barrier, &queue->tq_mutex, PWAIT, "-", 0); + TQ_SLEEP(queue, &t_barrier, "gtq_qdrain"); } /* @@ -296,32 +295,25 @@ gtaskqueue_drain_tq_queue(struct gtaskqueue *queue) static void gtaskqueue_drain_tq_active(struct gtaskqueue *queue) { - struct gtaskqueue_busy tb_marker, *tb_first; + struct gtaskqueue_busy *tb; + u_int seq; - if (TAILQ_EMPTY(&queue->tq_active)) + if (LIST_EMPTY(&queue->tq_active)) return; /* Block taskq_terminate().*/ queue->tq_callouts++; - /* - * Wait for all currently executing taskqueue threads - * to go idle. - */ - tb_marker.tb_running = TB_DRAIN_WAITER; - TAILQ_INSERT_TAIL(&queue->tq_active, &tb_marker, tb_link); - while (TAILQ_FIRST(&queue->tq_active) != &tb_marker) - TQ_SLEEP(queue, &tb_marker, &queue->tq_mutex, PWAIT, "-", 0); - TAILQ_REMOVE(&queue->tq_active, &tb_marker, tb_link); + /* Wait for any active task with sequence from the past. */ + seq = queue->tq_seq; +restart: + LIST_FOREACH(tb, &queue->tq_active, tb_link) { + if ((int)(tb->tb_seq - seq) <= 0) { + TQ_SLEEP(queue, tb->tb_running, "gtq_adrain"); + goto restart; + } + } - /* - * Wakeup any other drain waiter that happened to queue up - * without any intervening active thread. - */ - tb_first = TAILQ_FIRST(&queue->tq_active); - if (tb_first != NULL && tb_first->tb_running == TB_DRAIN_WAITER) - wakeup(tb_first); - /* Release taskqueue_terminate(). */ queue->tq_callouts--; if ((queue->tq_flags & TQ_FLAGS_ACTIVE) == 0) @@ -352,40 +344,27 @@ static void gtaskqueue_run_locked(struct gtaskqueue *queue) { struct gtaskqueue_busy tb; - struct gtaskqueue_busy *tb_first; struct gtask *gtask; KASSERT(queue != NULL, ("tq is NULL")); TQ_ASSERT_LOCKED(queue); tb.tb_running = NULL; + LIST_INSERT_HEAD(&queue->tq_active, &tb, tb_link); - while (STAILQ_FIRST(&queue->tq_queue)) { - TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link); - - /* - * Carefully remove the first task from the queue and - * clear its TASK_ENQUEUED flag - */ - gtask = STAILQ_FIRST(&queue->tq_queue); - KASSERT(gtask != NULL, ("task is NULL")); + while ((gtask = STAILQ_FIRST(&queue->tq_queue)) != NULL) { STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link); gtask->ta_flags &= ~TASK_ENQUEUED; tb.tb_running = gtask; + tb.tb_seq = ++queue->tq_seq; TQ_UNLOCK(queue); KASSERT(gtask->ta_func != NULL, ("task->ta_func is NULL")); gtask->ta_func(gtask->ta_context); TQ_LOCK(queue); - tb.tb_running = NULL; wakeup(gtask); - - TAILQ_REMOVE(&queue->tq_active, &tb, tb_link); - tb_first = TAILQ_FIRST(&queue->tq_active); - if (tb_first != NULL && - tb_first->tb_running == TB_DRAIN_WAITER) - wakeup(tb_first); } + LIST_REMOVE(&tb, tb_link); } static int @@ -394,7 +373,7 @@ task_is_running(struct gtaskqueue *queue, struct gtask struct gtaskqueue_busy *tb; TQ_ASSERT_LOCKED(queue); - TAILQ_FOREACH(tb, &queue->tq_active, tb_link) { + LIST_FOREACH(tb, &queue->tq_active, tb_link) { if (tb->tb_running == gtask) return (1); } @@ -427,7 +406,7 @@ static void gtaskqueue_drain_locked(struct gtaskqueue *queue, struct gtask *gtask) { while ((gtask->ta_flags & TASK_ENQUEUED) || task_is_running(queue, gtask)) - TQ_SLEEP(queue, gtask, &queue->tq_mutex, PWAIT, "-", 0); + TQ_SLEEP(queue, gtask, "gtq_drain"); } void @@ -563,7 +542,7 @@ gtaskqueue_thread_loop(void *arg) */ if ((tq->tq_flags & TQ_FLAGS_ACTIVE) == 0) break; - TQ_SLEEP(tq, tq, &tq->tq_mutex, 0, "-", 0); + TQ_SLEEP(tq, tq, "-"); } gtaskqueue_run_locked(tq); /* @@ -589,7 +568,7 @@ gtaskqueue_thread_enqueue(void *context) tqp = context; tq = *tqp; - wakeup_one(tq); + wakeup_any(tq); } Modified: head/sys/kern/subr_taskqueue.c ============================================================================== --- head/sys/kern/subr_taskqueue.c Fri Nov 1 21:26:43 2019 (r354240) +++ head/sys/kern/subr_taskqueue.c Fri Nov 1 22:49:44 2019 (r354241) @@ -56,24 +56,25 @@ static void taskqueue_swi_enqueue(void *); static void taskqueue_swi_giant_enqueue(void *); struct taskqueue_busy { - struct task *tb_running; - TAILQ_ENTRY(taskqueue_busy) tb_link; + struct task *tb_running; + u_int tb_seq; + LIST_ENTRY(taskqueue_busy) tb_link; }; -struct task * const TB_DRAIN_WAITER = (struct task *)0x1; - struct taskqueue { STAILQ_HEAD(, task) tq_queue; + LIST_HEAD(, taskqueue_busy) tq_active; + struct task *tq_hint; + u_int tq_seq; + int tq_callouts; + struct mtx_padalign tq_mutex; taskqueue_enqueue_fn tq_enqueue; void *tq_context; char *tq_name; - TAILQ_HEAD(, taskqueue_busy) tq_active; - struct mtx tq_mutex; struct thread **tq_threads; int tq_tcount; int tq_spin; int tq_flags; - int tq_callouts; taskqueue_callback_fn tq_callbacks[TASKQUEUE_NUM_CALLBACKS]; void *tq_cb_contexts[TASKQUEUE_NUM_CALLBACKS]; }; @@ -116,12 +117,11 @@ _timeout_task_init(struct taskqueue *queue, struct tim } static __inline int -TQ_SLEEP(struct taskqueue *tq, void *p, struct mtx *m, int pri, const char *wm, - int t) +TQ_SLEEP(struct taskqueue *tq, void *p, const char *wm) { if (tq->tq_spin) - return (msleep_spin(p, m, wm, t)); - return (msleep(p, m, pri, wm, t)); + return (msleep_spin(p, (struct mtx *)&tq->tq_mutex, wm, 0)); + return (msleep(p, &tq->tq_mutex, 0, wm, 0)); } static struct taskqueue * @@ -145,7 +145,7 @@ _taskqueue_create(const char *name, int mflags, snprintf(tq_name, TASKQUEUE_NAMELEN, "%s", (name) ? name : "taskqueue"); STAILQ_INIT(&queue->tq_queue); - TAILQ_INIT(&queue->tq_active); + LIST_INIT(&queue->tq_active); queue->tq_enqueue = enqueue; queue->tq_context = context; queue->tq_name = tq_name; @@ -196,7 +196,7 @@ taskqueue_terminate(struct thread **pp, struct taskque while (tq->tq_tcount > 0 || tq->tq_callouts > 0) { wakeup(tq); - TQ_SLEEP(tq, pp, &tq->tq_mutex, PWAIT, "taskqueue_destroy", 0); + TQ_SLEEP(tq, pp, "tq_destroy"); } } @@ -207,7 +207,7 @@ taskqueue_free(struct taskqueue *queue) TQ_LOCK(queue); queue->tq_flags &= ~TQ_FLAGS_ACTIVE; taskqueue_terminate(queue->tq_threads, queue); - KASSERT(TAILQ_EMPTY(&queue->tq_active), ("Tasks still running?")); + KASSERT(LIST_EMPTY(&queue->tq_active), ("Tasks still running?")); KASSERT(queue->tq_callouts == 0, ("Armed timeout tasks")); mtx_destroy(&queue->tq_mutex); free(queue->tq_threads, M_TASKQUEUE); @@ -233,21 +233,30 @@ taskqueue_enqueue_locked(struct taskqueue *queue, stru } /* - * Optimise the case when all tasks have the same priority. + * Optimise cases when all tasks use small set of priorities. + * In case of only one priority we always insert at the end. + * In case of two tq_hint typically gives the insertion point. + * In case of more then two tq_hint should halve the search. */ prev = STAILQ_LAST(&queue->tq_queue, task, ta_link); if (!prev || prev->ta_priority >= task->ta_priority) { STAILQ_INSERT_TAIL(&queue->tq_queue, task, ta_link); } else { - prev = NULL; - for (ins = STAILQ_FIRST(&queue->tq_queue); ins; - prev = ins, ins = STAILQ_NEXT(ins, ta_link)) + prev = queue->tq_hint; + if (prev && prev->ta_priority >= task->ta_priority) { + ins = STAILQ_NEXT(prev, ta_link); + } else { + prev = NULL; + ins = STAILQ_FIRST(&queue->tq_queue); + } + for (; ins; prev = ins, ins = STAILQ_NEXT(ins, ta_link)) if (ins->ta_priority < task->ta_priority) break; - if (prev) + if (prev) { STAILQ_INSERT_AFTER(&queue->tq_queue, prev, task, ta_link); - else + queue->tq_hint = task; + } else STAILQ_INSERT_HEAD(&queue->tq_queue, task, ta_link); } @@ -364,6 +373,7 @@ taskqueue_drain_tq_queue(struct taskqueue *queue) */ TASK_INIT(&t_barrier, USHRT_MAX, taskqueue_task_nop_fn, &t_barrier); STAILQ_INSERT_TAIL(&queue->tq_queue, &t_barrier, ta_link); + queue->tq_hint = &t_barrier; t_barrier.ta_pending = 1; /* @@ -371,7 +381,7 @@ taskqueue_drain_tq_queue(struct taskqueue *queue) * have completed or are currently executing. */ while (t_barrier.ta_pending != 0) - TQ_SLEEP(queue, &t_barrier, &queue->tq_mutex, PWAIT, "-", 0); + TQ_SLEEP(queue, &t_barrier, "tq_qdrain"); return (1); } @@ -383,32 +393,25 @@ taskqueue_drain_tq_queue(struct taskqueue *queue) static int taskqueue_drain_tq_active(struct taskqueue *queue) { - struct taskqueue_busy tb_marker, *tb_first; + struct taskqueue_busy *tb; + u_int seq; - if (TAILQ_EMPTY(&queue->tq_active)) + if (LIST_EMPTY(&queue->tq_active)) return (0); /* Block taskq_terminate().*/ queue->tq_callouts++; - /* - * Wait for all currently executing taskqueue threads - * to go idle. - */ - tb_marker.tb_running = TB_DRAIN_WAITER; - TAILQ_INSERT_TAIL(&queue->tq_active, &tb_marker, tb_link); - while (TAILQ_FIRST(&queue->tq_active) != &tb_marker) - TQ_SLEEP(queue, &tb_marker, &queue->tq_mutex, PWAIT, "-", 0); - TAILQ_REMOVE(&queue->tq_active, &tb_marker, tb_link); + /* Wait for any active task with sequence from the past. */ + seq = queue->tq_seq; +restart: + LIST_FOREACH(tb, &queue->tq_active, tb_link) { + if ((int)(tb->tb_seq - seq) <= 0) { + TQ_SLEEP(queue, tb->tb_running, "tq_adrain"); + goto restart; + } + } - /* - * Wakeup any other drain waiter that happened to queue up - * without any intervening active thread. - */ - tb_first = TAILQ_FIRST(&queue->tq_active); - if (tb_first != NULL && tb_first->tb_running == TB_DRAIN_WAITER) - wakeup(tb_first); - /* Release taskqueue_terminate(). */ queue->tq_callouts--; if ((queue->tq_flags & TQ_FLAGS_ACTIVE) == 0) @@ -440,42 +443,31 @@ static void taskqueue_run_locked(struct taskqueue *queue) { struct taskqueue_busy tb; - struct taskqueue_busy *tb_first; struct task *task; int pending; KASSERT(queue != NULL, ("tq is NULL")); TQ_ASSERT_LOCKED(queue); tb.tb_running = NULL; + LIST_INSERT_HEAD(&queue->tq_active, &tb, tb_link); - while (STAILQ_FIRST(&queue->tq_queue)) { - TAILQ_INSERT_TAIL(&queue->tq_active, &tb, tb_link); - - /* - * Carefully remove the first task from the queue and - * zero its pending count. - */ - task = STAILQ_FIRST(&queue->tq_queue); - KASSERT(task != NULL, ("task is NULL")); + while ((task = STAILQ_FIRST(&queue->tq_queue)) != NULL) { STAILQ_REMOVE_HEAD(&queue->tq_queue, ta_link); + if (queue->tq_hint == task) + queue->tq_hint = NULL; pending = task->ta_pending; task->ta_pending = 0; tb.tb_running = task; + tb.tb_seq = ++queue->tq_seq; TQ_UNLOCK(queue); KASSERT(task->ta_func != NULL, ("task->ta_func is NULL")); task->ta_func(task->ta_context, pending); TQ_LOCK(queue); - tb.tb_running = NULL; wakeup(task); - - TAILQ_REMOVE(&queue->tq_active, &tb, tb_link); - tb_first = TAILQ_FIRST(&queue->tq_active); - if (tb_first != NULL && - tb_first->tb_running == TB_DRAIN_WAITER) - wakeup(tb_first); } + LIST_REMOVE(&tb, tb_link); } void @@ -493,7 +485,7 @@ task_is_running(struct taskqueue *queue, struct task * struct taskqueue_busy *tb; TQ_ASSERT_LOCKED(queue); - TAILQ_FOREACH(tb, &queue->tq_active, tb_link) { + LIST_FOREACH(tb, &queue->tq_active, tb_link) { if (tb->tb_running == task) return (1); } @@ -522,8 +514,11 @@ taskqueue_cancel_locked(struct taskqueue *queue, struc u_int *pendp) { - if (task->ta_pending > 0) + if (task->ta_pending > 0) { STAILQ_REMOVE(&queue->tq_queue, task, task, ta_link); + if (queue->tq_hint == task) + queue->tq_hint = NULL; + } if (pendp != NULL) *pendp = task->ta_pending; task->ta_pending = 0; @@ -572,7 +567,7 @@ taskqueue_drain(struct taskqueue *queue, struct task * TQ_LOCK(queue); while (task->ta_pending != 0 || task_is_running(queue, task)) - TQ_SLEEP(queue, task, &queue->tq_mutex, PWAIT, "-", 0); + TQ_SLEEP(queue, task, "tq_drain"); TQ_UNLOCK(queue); } @@ -791,7 +786,7 @@ taskqueue_thread_loop(void *arg) */ if ((tq->tq_flags & TQ_FLAGS_ACTIVE) == 0) break; - TQ_SLEEP(tq, tq, &tq->tq_mutex, 0, "-", 0); + TQ_SLEEP(tq, tq, "-"); } taskqueue_run_locked(tq); /*