On Mon, 03 Jun 2024 17:44:27 +0100,
Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> 
> So my take on this is: not now!  I'm in the same area to improve the
> suspend-to-idle code that was recently committed.
> 
> Also, the diff does many things that should probably be separate
> changes.  Also, this may work for your machine, and for your use case,
> but it needs to work for everybody and across architectures.
>
> And while the idea of turning off cores that aren't used, in the end
> this depends on the scheduler to give you the right information.  And
> there are plans to make some drastic changes in that area as well.
>

I totaly agreed that this can be split into pices. The first and simplest
part is refactoring to extract sched_start_cpu / sched_stop_cpu, the next
and one and which brings some question I assume is parallel selection of
C-state for halt CPU.

I am on this list with hope to get some feedback regarding this work, and
make it cleaner and better at the end, becuase at least for my use case it
improves expirences and probably can be used by other.

I don't know well the kernel, nor all supported architecures, nor have
access to wariety of different hardware to test it.

And I really appricieted any feedback and help.

So, here the first pice with extracting sched_start_cpu / sched_stop_cpu:

diff --git sys/kern/kern_sched.c sys/kern/kern_sched.c
index 39d1250dab2..9c19be5b54c 100644
--- sys/kern/kern_sched.c
+++ sys/kern/kern_sched.c
@@ -635,6 +635,36 @@ sched_peg_curproc(struct cpu_info *ci)
 
 #ifdef MULTIPROCESSOR
 
+void
+sched_start_cpu(struct cpu_info *ci)
+{
+	struct schedstate_percpu *spc = &ci->ci_schedstate;
+
+	atomic_clearbits_int(&spc->spc_schedflags,
+		SPCF_SHOULDHALT | SPCF_HALTED);
+	cpuset_add(&sched_all_cpus, ci);
+}
+
+void sched_stop_cpu(struct cpu_info *ci)
+{
+	struct schedstate_percpu *spc = &ci->ci_schedstate;
+
+	cpuset_del(&sched_all_cpus, ci);
+	atomic_setbits_int(&spc->spc_schedflags, SPCF_SHOULDHALT);
+
+	need_resched(ci);
+}
+
+void sched_wait_halted_cpu(struct cpu_info *ci)
+{
+	struct schedstate_percpu *spc = &ci->ci_schedstate;
+
+	while ((spc->spc_schedflags & SPCF_HALTED) == 0) {
+		sleep_setup(spc, PZERO, "schedstate");
+		sleep_finish(0, (spc->spc_schedflags & SPCF_HALTED) == 0);
+	}
+}
+
 void
 sched_start_secondary_cpus(void)
 {
@@ -642,17 +672,13 @@ sched_start_secondary_cpus(void)
 	struct cpu_info *ci;
 
 	CPU_INFO_FOREACH(cii, ci) {
-		struct schedstate_percpu *spc = &ci->ci_schedstate;
-
 		if (CPU_IS_PRIMARY(ci) || !CPU_IS_RUNNING(ci))
 			continue;
-		atomic_clearbits_int(&spc->spc_schedflags,
-		    SPCF_SHOULDHALT | SPCF_HALTED);
 #ifdef __HAVE_CPU_TOPOLOGY
 		if (!sched_smt && ci->ci_smt_id > 0)
 			continue;
 #endif
-		cpuset_add(&sched_all_cpus, ci);
+		sched_start_cpu(ci);
 	}
 }
 
@@ -666,23 +692,14 @@ sched_stop_secondary_cpus(void)
 	 * Make sure we stop the secondary CPUs.
 	 */
 	CPU_INFO_FOREACH(cii, ci) {
-		struct schedstate_percpu *spc = &ci->ci_schedstate;
-
 		if (CPU_IS_PRIMARY(ci) || !CPU_IS_RUNNING(ci))
 			continue;
-		cpuset_del(&sched_all_cpus, ci);
-		atomic_setbits_int(&spc->spc_schedflags, SPCF_SHOULDHALT);
+		sched_stop_cpu(ci);
 	}
 	CPU_INFO_FOREACH(cii, ci) {
-		struct schedstate_percpu *spc = &ci->ci_schedstate;
-
 		if (CPU_IS_PRIMARY(ci) || !CPU_IS_RUNNING(ci))
 			continue;
-		while ((spc->spc_schedflags & SPCF_HALTED) == 0) {
-			sleep_setup(spc, PZERO, "schedstate");
-			sleep_finish(0,
-			    (spc->spc_schedflags & SPCF_HALTED) == 0);
-		}
+		sched_wait_halted_cpu(ci);
 	}
 }
 
@@ -879,9 +896,9 @@ sysctl_hwsmt(void *oldp, size_t *oldlenp, void *newp, size_t newlen)
 		if (ci->ci_smt_id == 0)
 			continue;
 		if (sched_smt)
-			cpuset_add(&sched_all_cpus, ci);
+			sched_start_cpu(ci);
 		else
-			cpuset_del(&sched_all_cpus, ci);
+			sched_stop_cpu(ci);
 	}
 
 	return 0;
diff --git sys/sys/sched.h sys/sys/sched.h
index dcc9b636679..31f43666323 100644
--- sys/sys/sched.h
+++ sys/sys/sched.h
@@ -177,6 +177,9 @@ int sysctl_hwsmt(void *, size_t *, void *, size_t);
 int sysctl_hwncpuonline(void);
 
 #ifdef MULTIPROCESSOR
+void sched_start_cpu(struct cpu_info *ci);
+void sched_stop_cpu(struct cpu_info *ci);
+void sched_wait_halted_cpu(struct cpu_info *ci);
 void sched_start_secondary_cpus(void);
 void sched_stop_secondary_cpus(void);
 #endif


-- 
wbr, Kirill