Module Name: src
Committed By: thorpej
Date: Sat Apr 25 00:07:27 UTC 2020
Modified Files:
src/sys/dev/ata: ata.c ata_subr.c atavar.h
Log Message:
Rather than creating a kthread-per-channel, use a threadpool and a
threadpool-job-per-channel for the in-thread-context work that needs
to be done (which is rare).
On one of my test systems, this results in the total number of LWPs
after multi-user boot dropping from 116 to 78.
To generate a diff of this commit:
cvs rdiff -u -r1.155 -r1.156 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.9 -r1.10 src/sys/dev/ata/ata_subr.c
cvs rdiff -u -r1.105 -r1.106 src/sys/dev/ata/atavar.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/dev/ata/ata.c
diff -u src/sys/dev/ata/ata.c:1.155 src/sys/dev/ata/ata.c:1.156
--- src/sys/dev/ata/ata.c:1.155 Mon Apr 13 10:49:34 2020
+++ src/sys/dev/ata/ata.c Sat Apr 25 00:07:27 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.155 2020/04/13 10:49:34 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.156 2020/04/25 00:07:27 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.155 2020/04/13 10:49:34 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.156 2020/04/25 00:07:27 thorpej Exp $");
#include "opt_ata.h"
@@ -425,47 +425,60 @@ atabusconfig_thread(void *arg)
}
/*
- * atabus_thread:
+ * atabus_tp_job:
*
* Worker thread for the ATA bus.
*/
static void
-atabus_thread(void *arg)
+atabus_tp_job(struct threadpool_job *j)
{
- struct atabus_softc *sc = arg;
- struct ata_channel *chp = sc->sc_chan;
+ struct ata_channel *chp =
+ container_of(j, struct ata_channel, ch_tp_job);
+ struct atabus_softc *sc = device_private(chp->atabus);
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));
+ chp->ch_job_thread = curlwp; /* XXX gross */
- /*
- * 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;
+ /* 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;
+ }
+ chp->ch_initial_config_done = true;
+ ata_channel_unlock(chp);
+ atabusconfig(sc);
+ ata_channel_lock(chp);
}
- ata_channel_unlock(chp);
- atabusconfig(sc);
+#define WORK_TO_DO \
+ (ATACH_TH_RESCAN | ATACH_TH_RESET | ATACH_TH_DRIVE_RESET | \
+ ATACH_TH_RECOVERY)
- ata_channel_lock(chp);
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);
- }
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);
@@ -534,10 +547,11 @@ atabus_thread(void *arg)
ata_channel_lock(chp);
}
}
- chp->ch_thread = NULL;
- cv_signal(&chp->ch_thr_idle);
+ chp->ch_job_thread = NULL; /* XXX gross */
+ threadpool_job_done(&chp->ch_tp_job);
ata_channel_unlock(chp);
- kthread_exit(0);
+
+ KERNEL_UNLOCK_ONE(NULL);
}
bool
@@ -545,7 +559,7 @@ ata_is_thread_run(struct ata_channel *ch
{
KASSERT(mutex_owned(&chp->ch_lock));
- return (chp->ch_thread == curlwp && !cpu_intr_p());
+ return (chp->ch_job_thread == curlwp && !cpu_intr_p());
}
static void
@@ -553,7 +567,7 @@ ata_thread_wake_locked(struct ata_channe
{
KASSERT(mutex_owned(&chp->ch_lock));
ata_channel_freeze_locked(chp);
- cv_signal(&chp->ch_thr_idle);
+ threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
}
/*
@@ -587,7 +601,6 @@ atabus_attach(device_t parent, device_t
struct atabus_softc *sc = device_private(self);
struct ata_channel *chp = aux;
struct atabus_initq *initq;
- int error;
sc->sc_chan = chp;
@@ -608,11 +621,15 @@ atabus_attach(device_t parent, device_t
mutex_exit(&atabus_qlock);
config_pending_incr(sc->sc_dev);
- /* 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);
+ /* 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);
if (!pmf_device_register(self, atabus_suspend, atabus_resume))
aprint_error_dev(self, "couldn't establish power handler\n");
@@ -666,13 +683,10 @@ atabus_detach(device_t self, int flags)
}
}
- /* Shutdown the channel. */
+ /* Cancel any in-progress job and dispose of the threadpool. */
ata_channel_lock(chp);
chp->ch_flags |= ATACH_SHUTDOWN;
- while (chp->ch_thread != NULL) {
- cv_signal(&chp->ch_thr_idle);
- cv_wait(&chp->ch_thr_idle, &chp->ch_lock);
- }
+ threadpool_cancel_job(chp->ch_tp, &chp->ch_tp_job);
ata_channel_unlock(chp);
atabus_free_drives(chp);
@@ -1588,7 +1602,7 @@ ata_thread_run(struct ata_channel *chp,
ata_channel_freeze_locked(chp);
chp->ch_flags |= type;
- cv_signal(&chp->ch_thr_idle);
+ threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
return;
}
@@ -1654,7 +1668,7 @@ ata_thread_run(struct ata_channel *chp,
ata_channel_thaw_locked(chp);
/* Signal the thread in case there is an xfer to run */
- cv_signal(&chp->ch_thr_idle);
+ threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
}
int
@@ -2285,7 +2299,7 @@ atabus_rescan(device_t self, const char
ata_channel_lock(chp);
chp->ch_flags |= ATACH_TH_RESCAN;
- cv_signal(&chp->ch_thr_idle);
+ threadpool_schedule_job(chp->ch_tp, &chp->ch_tp_job);
ata_channel_unlock(chp);
return 0;
Index: src/sys/dev/ata/ata_subr.c
diff -u src/sys/dev/ata/ata_subr.c:1.9 src/sys/dev/ata/ata_subr.c:1.10
--- src/sys/dev/ata/ata_subr.c:1.9 Sat Apr 4 22:30:02 2020
+++ src/sys/dev/ata/ata_subr.c Sat Apr 25 00:07:27 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: ata_subr.c,v 1.9 2020/04/04 22:30:02 jdolecek Exp $ */
+/* $NetBSD: ata_subr.c,v 1.10 2020/04/25 00:07:27 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.9 2020/04/04 22:30:02 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_subr.c,v 1.10 2020/04/25 00:07:27 thorpej Exp $");
#include "opt_ata.h"
@@ -195,8 +195,11 @@ ata_queue_free(struct ata_queue *chq)
void
ata_channel_init(struct ata_channel *chp)
{
+ int error __diagused;
+
mutex_init(&chp->ch_lock, MUTEX_DEFAULT, IPL_BIO);
- cv_init(&chp->ch_thr_idle, "atath");
+ error = threadpool_get(&chp->ch_tp, PRI_NONE);
+ KASSERT(error == 0); /* XXX */
callout_init(&chp->c_timo_callout, 0); /* XXX MPSAFE */
@@ -220,7 +223,7 @@ ata_channel_destroy(struct ata_channel *
mutex_exit(&chp->ch_lock);
mutex_destroy(&chp->ch_lock);
- cv_destroy(&chp->ch_thr_idle);
+ threadpool_put(chp->ch_tp, PRI_NONE);
}
void
Index: src/sys/dev/ata/atavar.h
diff -u src/sys/dev/ata/atavar.h:1.105 src/sys/dev/ata/atavar.h:1.106
--- src/sys/dev/ata/atavar.h:1.105 Mon Apr 13 10:49:34 2020
+++ src/sys/dev/ata/atavar.h Sat Apr 25 00:07:27 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.105 2020/04/13 10:49:34 jdolecek Exp $ */
+/* $NetBSD: atavar.h,v 1.106 2020/04/25 00:07:27 thorpej Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -28,6 +28,7 @@
#define _DEV_ATA_ATAVAR_H_
#include <sys/lock.h>
+#include <sys/threadpool.h>
#include <sys/queue.h>
#include <dev/ata/ataconf.h>
@@ -436,9 +437,14 @@ struct ata_channel {
*/
struct ata_queue *ch_queue;
- /* The channel kernel thread */
- struct lwp *ch_thread;
- kcondvar_t ch_thr_idle; /* thread waiting for work */
+ /*
+ * 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 */
/* Number of sata PMP ports, if any */
int ch_satapmp_nports;
