On 6/7/22, Alexander Motin <[email protected]> wrote:
> The branch main has been updated by mav:
>
> URL:
> https://cgit.FreeBSD.org/src/commit/?id=eff9ee7c0c8e1fe782d6c01a29bb23224b02a847
>
> commit eff9ee7c0c8e1fe782d6c01a29bb23224b02a847
> Author: Alexander Motin <[email protected]>
> AuthorDate: 2022-06-07 02:36:16 +0000
> Commit: Alexander Motin <[email protected]>
> CommitDate: 2022-06-07 02:51:01 +0000
>
> hwpmc: Increase thread priority while iterating CPUs.
>
> This allows to profile already running high-priority threads, that
> otherwise by blocking thread migration to respective CPUs blocked PMC
> management, i.e. profiling could start only when workload completed.
>
> While there, return the thread to its original CPU after iterating
> the list. Otherwise all threads using PMC end up on the last CPU.
>
the iteration is a bogus scheme and needs to be replaced with
something similar to what rms locks are doing right now, see rms_rlock
and rms_wlock. Maybe I'll hack it up this month.
> MFC after: 1 month
> ---
> sys/dev/hwpmc/hwpmc_logging.c | 21 +++++++++------------
> sys/dev/hwpmc/hwpmc_mod.c | 17 ++++++++---------
> sys/sys/pmc.h | 4 ++++
> 3 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/sys/dev/hwpmc/hwpmc_logging.c b/sys/dev/hwpmc/hwpmc_logging.c
> index c16adff8c842..8d9015af78e9 100644
> --- a/sys/dev/hwpmc/hwpmc_logging.c
> +++ b/sys/dev/hwpmc/hwpmc_logging.c
> @@ -764,6 +764,7 @@ pmclog_deconfigure_log(struct pmc_owner *po)
> {
> int error;
> struct pmclog_buffer *lb;
> + struct pmc_binding pb;
>
> PMCDBG1(LOG,CFG,1, "de-config po=%p", po);
>
> @@ -787,19 +788,16 @@ pmclog_deconfigure_log(struct pmc_owner *po)
> PMCLOG_RESET_BUFFER_DESCRIPTOR(lb);
> pmc_plb_rele(lb);
> }
> + pmc_save_cpu_binding(&pb);
> for (int i = 0; i < mp_ncpus; i++) {
> - thread_lock(curthread);
> - sched_bind(curthread, i);
> - thread_unlock(curthread);
> + pmc_select_cpu(i);
> /* return the 'current' buffer to the global pool */
> if ((lb = po->po_curbuf[curcpu]) != NULL) {
> PMCLOG_RESET_BUFFER_DESCRIPTOR(lb);
> pmc_plb_rele(lb);
> }
> }
> - thread_lock(curthread);
> - sched_unbind(curthread);
> - thread_unlock(curthread);
> + pmc_restore_cpu_binding(&pb);
>
> /* drop a reference to the fd */
> if (po->po_file != NULL) {
> @@ -869,18 +867,17 @@ pmclog_schedule_one_cond(struct pmc_owner *po)
> static void
> pmclog_schedule_all(struct pmc_owner *po)
> {
> + struct pmc_binding pb;
> +
> /*
> * Schedule the current buffer if any and not empty.
> */
> + pmc_save_cpu_binding(&pb);
> for (int i = 0; i < mp_ncpus; i++) {
> - thread_lock(curthread);
> - sched_bind(curthread, i);
> - thread_unlock(curthread);
> + pmc_select_cpu(i);
> pmclog_schedule_one_cond(po);
> }
> - thread_lock(curthread);
> - sched_unbind(curthread);
> - thread_unlock(curthread);
> + pmc_restore_cpu_binding(&pb);
> }
>
> int
> diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c
> index e94527041af8..18b8bb1674a3 100644
> --- a/sys/dev/hwpmc/hwpmc_mod.c
> +++ b/sys/dev/hwpmc/hwpmc_mod.c
> @@ -249,9 +249,6 @@ static void pmc_process_thread_delete(struct thread
> *td);
> static void pmc_process_thread_userret(struct thread *td);
> static void pmc_remove_owner(struct pmc_owner *po);
> static void pmc_remove_process_descriptor(struct pmc_process *pp);
> -static void pmc_restore_cpu_binding(struct pmc_binding *pb);
> -static void pmc_save_cpu_binding(struct pmc_binding *pb);
> -static void pmc_select_cpu(int cpu);
> static int pmc_start(struct pmc *pm);
> static int pmc_stop(struct pmc *pm);
> static int pmc_syscall_handler(struct thread *td, void *syscall_args);
> @@ -739,13 +736,14 @@ pmc_ri_to_classdep(struct pmc_mdep *md, int ri, int
> *adjri)
> * save the cpu binding of the current kthread
> */
>
> -static void
> +void
> pmc_save_cpu_binding(struct pmc_binding *pb)
> {
> PMCDBG0(CPU,BND,2, "save-cpu");
> thread_lock(curthread);
> pb->pb_bound = sched_is_bound(curthread);
> pb->pb_cpu = curthread->td_oncpu;
> + pb->pb_priority = curthread->td_priority;
> thread_unlock(curthread);
> PMCDBG1(CPU,BND,2, "save-cpu cpu=%d", pb->pb_cpu);
> }
> @@ -754,16 +752,16 @@ pmc_save_cpu_binding(struct pmc_binding *pb)
> * restore the cpu binding of the current thread
> */
>
> -static void
> +void
> pmc_restore_cpu_binding(struct pmc_binding *pb)
> {
> PMCDBG2(CPU,BND,2, "restore-cpu curcpu=%d restore=%d",
> curthread->td_oncpu, pb->pb_cpu);
> thread_lock(curthread);
> - if (pb->pb_bound)
> - sched_bind(curthread, pb->pb_cpu);
> - else
> + sched_bind(curthread, pb->pb_cpu);
> + if (!pb->pb_bound)
> sched_unbind(curthread);
> + sched_prio(curthread, pb->pb_priority);
> thread_unlock(curthread);
> PMCDBG0(CPU,BND,2, "restore-cpu done");
> }
> @@ -772,7 +770,7 @@ pmc_restore_cpu_binding(struct pmc_binding *pb)
> * move execution over the specified cpu and bind it there.
> */
>
> -static void
> +void
> pmc_select_cpu(int cpu)
> {
> KASSERT(cpu >= 0 && cpu < pmc_cpu_max(),
> @@ -784,6 +782,7 @@ pmc_select_cpu(int cpu)
>
> PMCDBG1(CPU,SEL,2, "select-cpu cpu=%d", cpu);
> thread_lock(curthread);
> + sched_prio(curthread, PRI_MIN);
> sched_bind(curthread, cpu);
> thread_unlock(curthread);
>
> diff --git a/sys/sys/pmc.h b/sys/sys/pmc.h
> index 372e77ecdee7..18c38ca36659 100644
> --- a/sys/sys/pmc.h
> +++ b/sys/sys/pmc.h
> @@ -997,6 +997,7 @@ struct pmc_cpu {
> struct pmc_binding {
> int pb_bound; /* is bound? */
> int pb_cpu; /* if so, to which CPU */
> + u_char pb_priority; /* Thread active priority. */
> };
>
> struct pmc_mdep;
> @@ -1225,6 +1226,9 @@ int pmc_save_kernel_callchain(uintptr_t *_cc, int
> _maxsamples,
> struct trapframe *_tf);
> int pmc_save_user_callchain(uintptr_t *_cc, int _maxsamples,
> struct trapframe *_tf);
> +void pmc_restore_cpu_binding(struct pmc_binding *pb);
> +void pmc_save_cpu_binding(struct pmc_binding *pb);
> +void pmc_select_cpu(int cpu);
> struct pmc_mdep *pmc_mdep_alloc(int nclasses);
> void pmc_mdep_free(struct pmc_mdep *md);
> uint64_t pmc_rdtsc(void);
>
--
Mateusz Guzik <mjguzik gmail.com>
