Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/external/bsd/common Avoid the prospect of callout calls ...



details:   https://anonhg.NetBSD.org/src/rev/2a4001d634f5
branches:  trunk
changeset: 835446:2a4001d634f5
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Mon Aug 27 15:05:01 2018 +0000

description:
Avoid the prospect of callout calls piling up.

Don't ever callout_schedule the callout while an existing call may be
in progress.

Echo some cases from cancel_delayed_work in flush_delayed_work.

diffstat:

 sys/external/bsd/common/include/linux/workqueue.h |    3 +-
 sys/external/bsd/common/linux/linux_work.c        |  136 +++++++++++++++------
 2 files changed, 99 insertions(+), 40 deletions(-)

diffs (275 lines):

diff -r aed26ea5a5ce -r 2a4001d634f5 sys/external/bsd/common/include/linux/workqueue.h
--- a/sys/external/bsd/common/include/linux/workqueue.h Mon Aug 27 15:04:45 2018 +0000
+++ b/sys/external/bsd/common/include/linux/workqueue.h Mon Aug 27 15:05:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: workqueue.h,v 1.11 2018/08/27 15:03:20 riastradh Exp $ */
+/*     $NetBSD: workqueue.h,v 1.12 2018/08/27 15:05:01 riastradh Exp $ */
 
 /*-
  * Copyright (c) 2013, 2018 The NetBSD Foundation, Inc.
@@ -72,6 +72,7 @@
        struct work_struct              work; /* Linux API name */
        struct callout                  dw_callout;
        TAILQ_ENTRY(delayed_work)       dw_entry;
+       int                             dw_resched;
        enum {
                DELAYED_WORK_IDLE,
                DELAYED_WORK_SCHEDULED,
diff -r aed26ea5a5ce -r 2a4001d634f5 sys/external/bsd/common/linux/linux_work.c
--- a/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:04:45 2018 +0000
+++ b/sys/external/bsd/common/linux/linux_work.c        Mon Aug 27 15:05:01 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: linux_work.c,v 1.34 2018/08/27 15:04:45 riastradh Exp $        */
+/*     $NetBSD: linux_work.c,v 1.35 2018/08/27 15:05:01 riastradh Exp $        */
 
 /*-
  * Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: linux_work.c,v 1.34 2018/08/27 15:04:45 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: linux_work.c,v 1.35 2018/08/27 15:05:01 riastradh Exp $");
 
 #include <sys/types.h>
 #include <sys/atomic.h>
@@ -314,7 +314,10 @@
                cv_broadcast(&wq->wq_cv);
                break;
        case DELAYED_WORK_RESCHEDULED:
+               KASSERT(dw->dw_resched >= 0);
+               callout_schedule(&dw->dw_callout, dw->dw_resched);
                dw->dw_state = DELAYED_WORK_SCHEDULED;
+               dw->dw_resched = -1;
                break;
        case DELAYED_WORK_CANCELLED:
                cancel_delayed_work_done(wq, dw);
@@ -546,6 +549,7 @@
 
        INIT_WORK(&dw->work, fn);
        dw->dw_state = DELAYED_WORK_IDLE;
+       dw->dw_resched = -1;
 
        /*
         * Defer callout_init until we are going to schedule the
@@ -599,6 +603,7 @@
 
        TAILQ_REMOVE(&wq->wq_delayed, dw, dw_entry);
        callout_destroy(&dw->dw_callout);
+       dw->dw_resched = -1;
        dw->dw_state = DELAYED_WORK_IDLE;
 }
 
@@ -723,8 +728,7 @@
                                 * cancelled.  Reschedule it.
                                 */
                                dw->dw_state = DELAYED_WORK_RESCHEDULED;
-                               callout_schedule(&dw->dw_callout,
-                                   MIN(INT_MAX, ticks));
+                               dw->dw_resched = MIN(INT_MAX, ticks);
                                break;
                        default:
                                panic("invalid delayed work state: %d",
@@ -867,17 +871,12 @@
                                         * the lock.
                                         */
                                } else {
-                                       /*
-                                        * Schedule callout and tell
-                                        * the instance that's running
-                                        * now that it's been
-                                        * rescheduled.
-                                        */
+                                       /* Ask the callout to reschedule.  */
                                        dw->dw_state = DELAYED_WORK_RESCHEDULED;
-                                       callout_schedule(&dw->dw_callout,
-                                           MIN(INT_MAX, ticks));
+                                       dw->dw_resched = MIN(INT_MAX, ticks);
                                }
                        } else {
+                               /* We stopped the callout before it began.  */
                                if (ticks == 0) {
                                        /*
                                         * Run immediately: destroy the
@@ -901,12 +900,31 @@
                        timer_modified = true;
                        break;
                case DELAYED_WORK_RESCHEDULED:
+                       /*
+                        * Someone rescheduled it after the callout
+                        * started but before the poor thing even had a
+                        * chance to acquire the lock.
+                        */
+                       if (ticks == 0) {
+                               /*
+                                * We can just switch back to
+                                * DELAYED_WORK_SCHEDULED so that the
+                                * callout will queue the work as soon
+                                * as it gets the lock.
+                                */
+                               dw->dw_state = DELAYED_WORK_SCHEDULED;
+                               dw->dw_resched = -1;
+                       } else {
+                               /* Change the rescheduled time.  */
+                               dw->dw_resched = ticks;
+                       }
+                       timer_modified = true;
+                       break;
                case DELAYED_WORK_CANCELLED:
                        /*
-                        * Someone modified the timer _again_, or
-                        * cancelled it, after the callout started but
-                        * before the poor thing even had a chance to
-                        * acquire the lock.
+                        * Someone cancelled it after the callout
+                        * started but before the poor thing even had a
+                        * chance to acquire the lock.
                         */
                        if (ticks == 0) {
                                /*
@@ -918,9 +936,8 @@
                                dw->dw_state = DELAYED_WORK_SCHEDULED;
                        } else {
                                /* Reschedule it.  */
-                               callout_schedule(&dw->dw_callout,
-                                   MIN(INT_MAX, ticks));
                                dw->dw_state = DELAYED_WORK_RESCHEDULED;
+                               dw->dw_resched = MIN(INT_MAX, ticks);
                        }
                        timer_modified = true;
                        break;
@@ -949,18 +966,36 @@
        } else {
                switch (dw->dw_state) {
                case DELAYED_WORK_IDLE:
-                       if (wq->wq_current_work == &dw->work) {
+                       /*
+                        * It is either on the queue or already running
+                        * or both.
+                        */
+                       if (wq->wq_current_work != &dw->work ||
+                           wq->wq_requeued) {
                                /*
-                                * Too late, it's already running.  If
-                                * it's been requeued, tough -- it'll
-                                * run again.
+                                * It is on the queue, and it may or
+                                * may not be running.  Remove it from
+                                * the queue.
+                                */
+                               TAILQ_REMOVE(&wq->wq_queue, &dw->work,
+                                   work_entry);
+                               if (wq->wq_current_work == &dw->work) {
+                                       /*
+                                        * If it is running, then it
+                                        * must have been requeued in
+                                        * this case, so mark it no
+                                        * longer requeued.
+                                        */
+                                       KASSERT(wq->wq_requeued);
+                                       wq->wq_requeued = false;
+                               }
+                               cancelled_p = true;
+                       } else {
+                               /*
+                                * Too late, it's already running, but
+                                * at least it hasn't been requeued.
                                 */
                                cancelled_p = false;
-                       } else {
-                               /* Got in before it started.  Remove it.  */
-                               TAILQ_REMOVE(&wq->wq_queue, &dw->work,
-                                   work_entry);
-                               cancelled_p = true;
                        }
                        break;
                case DELAYED_WORK_SCHEDULED:
@@ -986,6 +1021,7 @@
                         * already fired.  We must ask it to cancel.
                         */
                        dw->dw_state = DELAYED_WORK_CANCELLED;
+                       dw->dw_resched = -1;
                        cancelled_p = true;
                        break;
                case DELAYED_WORK_CANCELLED:
@@ -1023,6 +1059,10 @@
        } else {
                switch (dw->dw_state) {
                case DELAYED_WORK_IDLE:
+                       /*
+                        * It is either on the queue or already running
+                        * or both.
+                        */
                        if (wq->wq_current_work == &dw->work) {
                                /*
                                 * Too late, it's already running.
@@ -1055,7 +1095,7 @@
                         * cancelled it in that case.
                         *
                         * If we stopped the callout before it started,
-                        * however, then destroy the callout and
+                        * then we must destroy the callout and
                         * dissociate it from the workqueue ourselves.
                         */
                        dw->dw_state = DELAYED_WORK_CANCELLED;
@@ -1070,6 +1110,7 @@
                         * wait for it to complete.
                         */
                        dw->dw_state = DELAYED_WORK_CANCELLED;
+                       dw->dw_resched = -1;
                        (void)callout_halt(&dw->dw_callout, &wq->wq_lock);
                        cancelled_p = true;
                        break;
@@ -1178,24 +1219,41 @@
                        flush_workqueue_locked(wq);
                        break;
                case DELAYED_WORK_SCHEDULED:
-               case DELAYED_WORK_RESCHEDULED:
-               case DELAYED_WORK_CANCELLED:
                        /*
-                        * The callout is still scheduled to run.
-                        * Notify it that we are cancelling, and try to
-                        * stop the callout before it runs.
+                        * If it is scheduled, mark it cancelled and
+                        * try to stop the callout before it starts.
                         *
-                        * If we do stop the callout, we are now
-                        * responsible for dissociating the work from
-                        * the queue.
+                        * If it's too late and the callout has already
+                        * begun to execute, we must wait for it to
+                        * complete.  But we got in soon enough to ask
+                        * the callout not to run.
                         *
-                        * Otherwise, wait for it to complete and
-                        * dissociate itself -- it will not put itself
-                        * on the workqueue once it is cancelled.
+                        * If we stopped the callout before it started,
+                        * then we must destroy the callout and
+                        * dissociate it from the workqueue ourselves.
                         */
                        dw->dw_state = DELAYED_WORK_CANCELLED;
                        if (!callout_halt(&dw->dw_callout, &wq->wq_lock))
                                cancel_delayed_work_done(wq, dw);
+                       break;
+               case DELAYED_WORK_RESCHEDULED:
+                       /*
+                        * If it is being rescheduled, the callout has
+                        * already fired.  We must ask it to cancel and
+                        * wait for it to complete.
+                        */
+                       dw->dw_state = DELAYED_WORK_CANCELLED;
+                       dw->dw_resched = -1;
+                       (void)callout_halt(&dw->dw_callout, &wq->wq_lock);
+                       break;
+               case DELAYED_WORK_CANCELLED:
+                       /*
+                        * If it is being cancelled, the callout has
+                        * already fired.  We need only wait for it to
+                        * complete.
+                        */
+                       (void)callout_halt(&dw->dw_callout, &wq->wq_lock);
+                       break;
                default:
                        panic("invalid delayed work state: %d",
                            dw->dw_state);



Home | Main Index | Thread Index | Old Index