Source-Changes-HG archive

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

[src/trunk]: src/sys/dev/ata Back out changes to use a threadpool for now; it...



details:   https://anonhg.NetBSD.org/src/rev/1575e714c627
branches:  trunk
changeset: 932232:1575e714c627
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Sat May 02 19:09:56 2020 +0000

description:
Back out changes to use a threadpool for now; it's causing trouble
for some folks on Thinkpads.

diffstat:

 sys/dev/ata/ata.c      |  108 +++++++++++++++++++++---------------------------
 sys/dev/ata/ata_subr.c |   11 +---
 sys/dev/ata/atavar.h   |   14 +----
 3 files changed, 55 insertions(+), 78 deletions(-)

diffs (285 lines):

diff -r 4b7d06ae614b -r 1575e714c627 sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Sat May 02 19:01:08 2020 +0000
+++ b/sys/dev/ata/ata.c Sat May 02 19:09:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata.c,v 1.156 2020/04/25 00:07:27 thorpej Exp $        */
+/*     $NetBSD: ata.c,v 1.157 2020/05/02 19:09:56 thorpej Exp $        */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.156 2020/04/25 00:07:27 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.157 2020/05/02 19:09:56 thorpej Exp $");
 
 #include "opt_ata.h"
 
@@ -425,60 +425,47 @@
 }
 
 /*
- * atabus_tp_job:
+ * atabus_thread:
  *
  *     Worker thread for the ATA bus.
  */
 static void
-atabus_tp_job(struct threadpool_job *j)
+atabus_thread(void *arg)
 {
-       struct ata_channel *chp =
-           container_of(j, struct ata_channel, ch_tp_job);
-       struct atabus_softc *sc = device_private(chp->atabus);
+       struct atabus_softc *sc = arg;
+       struct ata_channel *chp = sc->sc_chan;
        struct ata_queue *chq = chp->ch_queue;
        struct ata_xfer *xfer;
        int i, rv;
 
-       /* XXX MPSAFE?? */
-       KERNEL_LOCK(1, NULL);
+       ata_channel_lock(chp);
+       KASSERT(ata_is_thread_run(chp));
+
+       /*
+        * Probe the drives.  Reset type to indicate to controllers
+        * that can re-probe that all drives must be probed..
+        *
+        * Note: ch_ndrives may be changed during the probe.
+        */
+       KASSERT(chp->ch_ndrives == 0 || chp->ch_drive != NULL);
+       for (i = 0; i < chp->ch_ndrives; i++) {
+               chp->ch_drive[i].drive_flags = 0;
+               chp->ch_drive[i].drive_type = ATA_DRIVET_NONE;
+       }
+       ata_channel_unlock(chp);
+
+       atabusconfig(sc);
 
        ata_channel_lock(chp);
-       chp->ch_job_thread = curlwp;    /* XXX gross */
-
-                                       /* XXX gross */
-       if (__predict_false(!chp->ch_initial_config_done)) {
-               /*
-                * Probe the drives.  Reset type to indicate to controllers
-                * that can re-probe that all drives must be probed..
-                *
-                * Note: ch_ndrives may be changed during the probe.
-                */
-               KASSERT(chp->ch_ndrives == 0 || chp->ch_drive != NULL);
-               for (i = 0; i < chp->ch_ndrives; i++) {
-                       chp->ch_drive[i].drive_flags = 0;
-                       chp->ch_drive[i].drive_type = ATA_DRIVET_NONE;
+       for (;;) {
+               if ((chp->ch_flags & (ATACH_TH_RESET | ATACH_TH_DRIVE_RESET
+                   | ATACH_TH_RECOVERY | ATACH_SHUTDOWN)) == 0 &&
+                   (chq->queue_active == 0 || chq->queue_freeze == 0)) {
+                       cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
                }
-               chp->ch_initial_config_done = true;
-               ata_channel_unlock(chp);
-               atabusconfig(sc);
-               ata_channel_lock(chp);
-       }
-
-#define        WORK_TO_DO                                                      \
-       (ATACH_TH_RESCAN | ATACH_TH_RESET | ATACH_TH_DRIVE_RESET |      \
-        ATACH_TH_RECOVERY)
-
-       for (;;) {
                if (chp->ch_flags & ATACH_SHUTDOWN) {
                        break;
                }
-               if ((chp->ch_flags & WORK_TO_DO) == 0 &&
-                   (chq->queue_active == 0 || chq->queue_freeze == 0)) {
-                       break;
-               }
-
-#undef WORK_TO_DO
-
                if (chp->ch_flags & ATACH_TH_RESCAN) {
                        chp->ch_flags &= ~ATACH_TH_RESCAN;
                        ata_channel_unlock(chp);
@@ -547,11 +534,10 @@
                        ata_channel_lock(chp);
                }
        }
-       chp->ch_job_thread = NULL;      /* XXX gross */
-       threadpool_job_done(&chp->ch_tp_job);
+       chp->ch_thread = NULL;
+       cv_signal(&chp->ch_thr_idle);
        ata_channel_unlock(chp);
-
-       KERNEL_UNLOCK_ONE(NULL);
+       kthread_exit(0);
 }
 
 bool
@@ -559,7 +545,7 @@
 {
        KASSERT(mutex_owned(&chp->ch_lock));
 
-       return (chp->ch_job_thread == curlwp && !cpu_intr_p());
+       return (chp->ch_thread == curlwp && !cpu_intr_p());
 }
 
 static void
@@ -567,7 +553,7 @@
 {
        KASSERT(mutex_owned(&chp->ch_lock));
        ata_channel_freeze_locked(chp);
-       threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
+       cv_signal(&chp->ch_thr_idle);
 }
 
 /*
@@ -601,6 +587,7 @@
        struct atabus_softc *sc = device_private(self);
        struct ata_channel *chp = aux;
        struct atabus_initq *initq;
+       int error;
 
        sc->sc_chan = chp;
 
@@ -621,15 +608,11 @@
        mutex_exit(&atabus_qlock);
        config_pending_incr(sc->sc_dev);
 
-       /* Initialize our threadpool job. */
-       threadpool_job_init(&chp->ch_tp_job, atabus_tp_job,
-           &chp->ch_lock, "%s", device_xname(self));
-
-       /* ...and run it now to scan the bus. */
-       KASSERT(chp->ch_tp != NULL);
-       ata_channel_lock(chp);
-       threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
-       ata_channel_unlock(chp);
+       /* XXX MPSAFE - no KTHREAD_MPSAFE, so protected by KERNEL_LOCK() */
+       if ((error = kthread_create(PRI_NONE, 0, NULL, atabus_thread, sc,
+           &chp->ch_thread, "%s", device_xname(self))) != 0)
+               aprint_error_dev(self,
+                   "unable to create kernel thread: error %d\n", error);
 
        if (!pmf_device_register(self, atabus_suspend, atabus_resume))
                aprint_error_dev(self, "couldn't establish power handler\n");
@@ -683,10 +666,13 @@
                }
        }
 
-       /* Cancel any in-progress job and dispose of the threadpool. */
+       /* Shutdown the channel. */
        ata_channel_lock(chp);
        chp->ch_flags |= ATACH_SHUTDOWN;
-       threadpool_cancel_job(chp->ch_tp, &chp->ch_tp_job);
+       while (chp->ch_thread != NULL) {
+               cv_signal(&chp->ch_thr_idle);
+               cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
+       }
        ata_channel_unlock(chp);
 
        atabus_free_drives(chp);
@@ -1602,7 +1588,7 @@
                ata_channel_freeze_locked(chp);
                chp->ch_flags |= type;
 
-               threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
+               cv_signal(&chp->ch_thr_idle);
                return;
        }
 
@@ -1668,7 +1654,7 @@
        ata_channel_thaw_locked(chp);
 
        /* Signal the thread in case there is an xfer to run */
-       threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
+       cv_signal(&chp->ch_thr_idle);
 }
 
 int
@@ -2299,7 +2285,7 @@
 
        ata_channel_lock(chp);
        chp->ch_flags |= ATACH_TH_RESCAN;
-       threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
+       cv_signal(&chp->ch_thr_idle);
        ata_channel_unlock(chp);
 
        return 0;
diff -r 4b7d06ae614b -r 1575e714c627 sys/dev/ata/ata_subr.c
--- a/sys/dev/ata/ata_subr.c    Sat May 02 19:01:08 2020 +0000
+++ b/sys/dev/ata/ata_subr.c    Sat May 02 19:09:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ata_subr.c,v 1.10 2020/04/25 00:07:27 thorpej Exp $    */
+/*     $NetBSD: ata_subr.c,v 1.11 2020/05/02 19:09:56 thorpej Exp $    */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.  All rights reserved.
@@ -25,7 +25,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_subr.c,v 1.10 2020/04/25 00:07:27 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_subr.c,v 1.11 2020/05/02 19:09:56 thorpej Exp $");
 
 #include "opt_ata.h"
 
@@ -195,11 +195,8 @@
 void
 ata_channel_init(struct ata_channel *chp)
 {
-       int error __diagused;
-
        mutex_init(&chp->ch_lock, MUTEX_DEFAULT, IPL_BIO);
-       error = threadpool_get(&chp->ch_tp, PRI_NONE);
-       KASSERT(error == 0);                    /* XXX */
+       cv_init(&chp->ch_thr_idle, "atath");
 
        callout_init(&chp->c_timo_callout, 0);  /* XXX MPSAFE */
 
@@ -223,7 +220,7 @@
        mutex_exit(&chp->ch_lock);
 
        mutex_destroy(&chp->ch_lock);
-       threadpool_put(chp->ch_tp, PRI_NONE);
+       cv_destroy(&chp->ch_thr_idle);
 }
 
 void
diff -r 4b7d06ae614b -r 1575e714c627 sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h      Sat May 02 19:01:08 2020 +0000
+++ b/sys/dev/ata/atavar.h      Sat May 02 19:09:56 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: atavar.h,v 1.106 2020/04/25 00:07:27 thorpej Exp $     */
+/*     $NetBSD: atavar.h,v 1.107 2020/05/02 19:09:56 thorpej Exp $     */
 
 /*
  * Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -28,7 +28,6 @@
 #define        _DEV_ATA_ATAVAR_H_
 
 #include <sys/lock.h>
-#include <sys/threadpool.h>
 #include <sys/queue.h>
 
 #include <dev/ata/ataconf.h>
@@ -437,14 +436,9 @@
         */
        struct ata_queue *ch_queue;
 
-       /*
-        * Threadpool job scheduled whenever we need special work done in
-        * thread context.
-        */
-       struct threadpool *ch_tp;
-       struct threadpool_job ch_tp_job;
-       bool ch_initial_config_done;    /* XXX gross */
-       struct lwp *ch_job_thread;      /* XXX gross */
+       /* The channel kernel thread */
+       struct lwp *ch_thread;
+       kcondvar_t ch_thr_idle;         /* thread waiting for work */
 
        /* Number of sata PMP ports, if any */
        int ch_satapmp_nports;



Home | Main Index | Thread Index | Old Index