From owner-freebsd-arch@FreeBSD.ORG Sat Mar 17 06:52:53 2007 Return-Path: X-Original-To: arch@freebsd.org Delivered-To: freebsd-arch@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 5BB3C16A405 for ; Sat, 17 Mar 2007 06:52:53 +0000 (UTC) (envelope-from nate@root.org) Received: from root.org (root.org [67.118.192.226]) by mx1.freebsd.org (Postfix) with ESMTP id 42B7113C480 for ; Sat, 17 Mar 2007 06:52:53 +0000 (UTC) (envelope-from nate@root.org) Received: (qmail 9979 invoked from network); 17 Mar 2007 06:52:55 -0000 Received: from pm9-195.corp.redshift.com (HELO ?192.168.1.50?) (nate-mail@216.228.25.195) by root.org with ESMTPA; 17 Mar 2007 06:52:55 -0000 Message-ID: <45FB908A.6070006@root.org> Date: Fri, 16 Mar 2007 23:54:02 -0700 From: Nate Lawson User-Agent: Thunderbird 1.5.0.9 (X11/20070214) MIME-Version: 1.0 To: arch@freebsd.org X-Enigmail-Version: 0.94.2.0 Content-Type: multipart/mixed; boundary="------------010201050001060005050802" Cc: Subject: PATCH: cpu freq notifiers and eventhandler register from SYSINIT X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Mar 2007 06:52:53 -0000 This is a multi-part message in MIME format. --------------010201050001060005050802 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Attached is an updated patch for handling cpu freq changes. It updates tsc_freq and all direct consumers of it. I also would like to add something to eventhandler.h. As I was doing this, I really wanted a way to declare an eventhandler tag and call eventhandler_register() from a SYSINIT. I ended up doing that manually in a lot of cases where I couldn't register up front because it was too early in boot (i.e. identcpu). Comments on adding this macro, similar to TASQUEUE_DEFINE in taskqueue.h? Maybe EVENTHANDLER_REGISTER_INIT()? I wasn't sure what to do for guprof since it is using TSC at times. I decided to just have it print a warning if the TSC freq changed while profiling was active. Let me know if I should do something else here. For altq, it should be correct but please check my change. I'm not sure what to do for packets that are being processed if machclk_freq changes. It might be ok. Thanks, -- Nate --------------010201050001060005050802 Content-Type: text/plain; name="cpu_event.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="cpu_event.diff" Index: sys/i386/i386/tsc.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/tsc.c,v retrieving revision 1.206 diff -u -r1.206 tsc.c --- sys/i386/i386/tsc.c 4 Aug 2006 07:56:35 -0000 1.206 +++ sys/i386/i386/tsc.c 17 Mar 2007 01:25:14 -0000 @@ -30,6 +30,8 @@ #include "opt_clock.h" #include +#include +#include #include #include #include @@ -52,14 +54,20 @@ TUNABLE_INT("kern.timecounter.smp_tsc", &smp_tsc); #endif +static eventhandler_tag evh_pre_tag, evh_post_tag; + static unsigned tsc_get_timecount(struct timecounter *tc); +static void tsc_freq_changing(void *arg, const struct cf_level *level, + int *status); +static void tsc_freq_changed(void *arg, const struct cf_level *level, + int status); static struct timecounter tsc_timecounter = { tsc_get_timecount, /* get_timecount */ 0, /* no poll_pps */ - ~0u, /* counter_mask */ + ~0u, /* counter_mask */ 0, /* frequency */ - "TSC", /* name */ + "TSC", /* name */ 800, /* quality (adjusted in code) */ }; @@ -87,8 +95,13 @@ if (bootverbose) printf("TSC clock: %ju Hz\n", (intmax_t)tsc_freq); set_cputicker(rdtsc, tsc_freq, 1); -} + /* Register to find out about changes in CPU frequency. */ + evh_pre_tag = EVENTHANDLER_REGISTER(cpufreq_pre_change, + tsc_freq_changing, NULL, EVENTHANDLER_PRI_FIRST); + evh_post_tag = EVENTHANDLER_REGISTER(cpufreq_post_change, + tsc_freq_changed, NULL, EVENTHANDLER_PRI_FIRST); +} void init_TSC_tc(void) @@ -128,6 +141,38 @@ } } +/* + * If the TSC timecounter is in use, veto the pending change. It may be + * possible in the future to handle a dynamically-changing timecounter rate. + */ +static void +tsc_freq_changing(void *arg, const struct cf_level *level, int *status) +{ + static int once; + + if (*status == 0 && timecounter == &tsc_timecounter) { + if (!once) { + printf("timecounter TSC must not be in use when " + "changing frequencies; change denied\n"); + once = 1; + } + *status = EBUSY; + } +} + +/* Update TSC freq with the value indicated by the caller. */ +static void +tsc_freq_changed(void *arg, const struct cf_level *level, int status) +{ + /* If there was an error during the transition, don't do anything. */ + if (status != 0) + return; + + /* Total setting for this level gives the new frequency in MHz. */ + tsc_freq = level->total_set.freq * 1000000; + tsc_timecounter.tc_frequency = tsc_freq; +} + static int sysctl_machdep_tsc_freq(SYSCTL_HANDLER_ARGS) { Index: sys/i386/i386/identcpu.c =================================================================== RCS file: /home/ncvs/src/sys/i386/i386/identcpu.c,v retrieving revision 1.172 diff -u -r1.172 identcpu.c --- sys/i386/i386/identcpu.c 12 Mar 2007 20:27:21 -0000 1.172 +++ sys/i386/i386/identcpu.c 17 Mar 2007 06:30:34 -0000 @@ -45,6 +45,8 @@ #include #include +#include +#include #include #include #include @@ -101,6 +103,9 @@ &hw_clockrate, 0, "CPU instruction clock rate"); static char cpu_brand[48]; +static eventhandler_tag evh_post_tag; +static void tsc_freq_changed(void *arg, const struct cf_level *level, + int status); #define MAX_BRAND_INDEX 8 @@ -1077,6 +1082,29 @@ write_eflags(eflags); } +/* XXX Register with sysinit should be a macro in eventhandler.h */ +static void +ident_tsc_check(void *arg) +{ + + evh_post_tag = EVENTHANDLER_REGISTER(cpufreq_post_change, + tsc_freq_changed, NULL, EVENTHANDLER_PRI_ANY); +} +SYSINIT(ident_tsc_check, SI_SUB_CONFIGURE, SI_ORDER_ANY, ident_tsc_check, + NULL); + +/* Update TSC freq with the value indicated by the caller. */ +static void +tsc_freq_changed(void *arg, const struct cf_level *level, int status) +{ + /* If there was an error during the transition, don't do anything. */ + if (status != 0) + return; + + /* Total setting for this level gives the new frequency in MHz. */ + hw_clockrate = level->total_set.freq; +} + /* * Final stage of CPU identification. -- Should I check TI? */ Index: sys/kern/kern_cpu.c =================================================================== RCS file: /home/ncvs/src/sys/kern/kern_cpu.c,v retrieving revision 1.23 diff -u -r1.23 kern_cpu.c --- sys/kern/kern_cpu.c 3 Mar 2006 02:06:04 -0000 1.23 +++ sys/kern/kern_cpu.c 17 Mar 2007 01:23:40 -0000 @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2004-2005 Nate Lawson (SDG) + * Copyright (c) 2004-2007 Nate Lawson (SDG) * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -95,7 +95,6 @@ static int cpufreq_attach(device_t dev); static int cpufreq_detach(device_t dev); -static void cpufreq_evaluate(void *arg); static int cf_set_method(device_t dev, const struct cf_level *level, int priority); static int cf_get_method(device_t dev, struct cf_level *level); @@ -127,8 +126,6 @@ static devclass_t cpufreq_dc; DRIVER_MODULE(cpufreq, cpu, cpufreq_driver, cpufreq_dc, 0, 0); -static eventhandler_tag cf_ev_tag; - static int cf_lowest_freq; static int cf_verbose; TUNABLE_INT("debug.cpufreq.lowest", &cf_lowest_freq); @@ -176,8 +173,6 @@ SYSCTL_CHILDREN(device_get_sysctl_tree(parent)), OID_AUTO, "freq_levels", CTLTYPE_STRING | CTLFLAG_RD, sc, 0, cpufreq_levels_sysctl, "A", "CPU frequency levels"); - cf_ev_tag = EVENTHANDLER_REGISTER(cpufreq_changed, cpufreq_evaluate, - NULL, EVENTHANDLER_PRI_ANY); return (0); } @@ -202,18 +197,11 @@ numdevs = devclass_get_count(cpufreq_dc); if (numdevs == 1) { CF_DEBUG("final shutdown for %s\n", device_get_nameunit(dev)); - EVENTHANDLER_DEREGISTER(cpufreq_changed, cf_ev_tag); } return (0); } -static void -cpufreq_evaluate(void *arg) -{ - /* TODO: Re-evaluate when notified of changes to drivers. */ -} - static int cf_set_method(device_t dev, const struct cf_level *level, int priority) { @@ -222,26 +210,16 @@ struct cf_saved_freq *saved_freq, *curr_freq; struct pcpu *pc; int cpu_id, error, i; - static int once; sc = device_get_softc(dev); error = 0; set = NULL; saved_freq = NULL; - /* - * Check that the TSC isn't being used as a timecounter. - * If it is, then return EBUSY and refuse to change the - * clock speed. - */ - if (strcmp(timecounter->tc_name, "TSC") == 0) { - if (!once) { - printf("cpufreq: frequency change with timecounter" - " TSC not allowed, see cpufreq(4)\n"); - once = 1; - } - return (EBUSY); - } + /* We are going to change levels so notify the pre-change handler. */ + EVENTHANDLER_INVOKE(cpufreq_pre_change, level, &error); + if (error != 0) + return (error); CF_MTX_LOCK(&sc->lock); @@ -378,8 +356,15 @@ out: CF_MTX_UNLOCK(&sc->lock); + + /* + * We changed levels (or attempted to) so notify the post-change + * handler of new frequency or error. + */ + EVENTHANDLER_INVOKE(cpufreq_post_change, level, error); if (error && set) device_printf(set->dev, "set freq failed, err %d\n", error); + return (error); } Index: sys/sys/cpu.h =================================================================== RCS file: /home/ncvs/src/sys/sys/cpu.h,v retrieving revision 1.3 diff -u -r1.3 cpu.h --- sys/sys/cpu.h 19 Feb 2005 06:13:25 -0000 1.3 +++ sys/sys/cpu.h 17 Mar 2007 01:44:18 -0000 @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2005 Nate Lawson (SDG) + * Copyright (c) 2005-2007 Nate Lawson (SDG) * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -29,6 +29,8 @@ #ifndef _SYS_CPU_H_ #define _SYS_CPU_H_ +#include + /* * CPU device support. */ @@ -118,6 +120,12 @@ int cpufreq_register(device_t dev); int cpufreq_unregister(device_t dev); +/* Eventhandlers that are called before and after a change in frequency */ +typedef void (*cpufreq_pre_notify_fn)(void *, const struct cf_level *, int *); +typedef void (*cpufreq_post_notify_fn)(void *, const struct cf_level *, int); +EVENTHANDLER_DECLARE(cpufreq_pre_change, cpufreq_pre_notify_fn); +EVENTHANDLER_DECLARE(cpufreq_post_change, cpufreq_post_notify_fn); + /* Allow values to be +/- a bit since sometimes we have to estimate. */ #define CPUFREQ_CMP(x, y) (abs((x) - (y)) < 25) Index: sys/contrib/altq/altq/altq_subr.c =================================================================== RCS file: /home/ncvs/src/sys/contrib/altq/altq/altq_subr.c,v retrieving revision 1.8 diff -u -r1.8 altq_subr.c --- sys/contrib/altq/altq/altq_subr.c 2 Mar 2006 00:51:39 -0000 1.8 +++ sys/contrib/altq/altq/altq_subr.c 17 Mar 2007 02:10:01 -0000 @@ -74,6 +74,9 @@ #if __FreeBSD__ < 3 #include "opt_cpu.h" /* for FreeBSD-2.2.8 to get i586_ctr_freq */ #endif +#include +#include +#include #include #endif #if defined(__i386__) @@ -898,6 +901,22 @@ extern u_int64_t cpu_tsc_freq; #endif /* __alpha__ */ +#if defined(__FreeBSD__) && (__FreeBSD_version > 700030) +static eventhandler_tag evh_post_tag; + +/* Update TSC freq with the value indicated by the caller. */ +static void +tsc_freq_changed(void *arg, const struct cf_level *level, int status) +{ + /* If there was an error during the transition, don't do anything. */ + if (status != 0) + return; + + /* Total setting for this level gives the new frequency in MHz. */ + machclk_freq = level->total_set.freq * 1000000; +} +#endif /* __FreeBSD_version > 700030 */ + void init_machclk(void) { @@ -941,6 +960,11 @@ #ifdef __FreeBSD__ #if (__FreeBSD_version > 300000) machclk_freq = tsc_freq; +#if (__FreeBSD_version > 700030) + /* Register to see any changes in TSC freq. */ + evh_post_tag = EVENTHANDLER_REGISTER(cpufreq_post_change, + tsc_freq_changed, NULL, EVENTHANDLER_PRI_ANY); +#endif #else machclk_freq = i586_ctr_freq; #endif Index: sys/i386/isa/prof_machdep.c =================================================================== RCS file: /home/ncvs/src/sys/i386/isa/prof_machdep.c,v retrieving revision 1.29 diff -u -r1.29 prof_machdep.c --- sys/i386/isa/prof_machdep.c 29 Oct 2006 09:48:44 -0000 1.29 +++ sys/i386/isa/prof_machdep.c 17 Mar 2007 06:06:17 -0000 @@ -33,6 +33,9 @@ #include #include +#include +#include +#include #include #include #include @@ -50,6 +53,7 @@ int cputime_bias = 1; /* initialize for locality of reference */ +static int prof_active; static int cputime_clock = CPUTIME_CLOCK_UNINITIALIZED; #if defined(PERFMON) && defined(I586_PMC_GUPROF) static u_int cputime_clock_pmc_conf = I586_PMC_GUPROF; @@ -58,6 +62,10 @@ #endif #endif /* GUPROF */ +static eventhandler_tag evh_post_tag; +static void tsc_freq_changed(void *arg, const struct cf_level *level, + int status); + #ifdef __GNUCLIKE_ASM __asm(" \n\ GM_STATE = 0 \n\ @@ -287,7 +295,7 @@ if (cputime_clock == CPUTIME_CLOCK_UNINITIALIZED) { cputime_clock = CPUTIME_CLOCK_I8254; #if defined(I586_CPU) || defined(I686_CPU) - if (tsc_freq != 0 && !tsc_is_broken && mp_ncpus < 2) + if (tsc_freq != 0 && !tsc_is_broken && mp_ncpus == 1) cputime_clock = CPUTIME_CLOCK_TSC; #endif } @@ -322,6 +330,7 @@ } #endif /* PERFMON && I586_PMC_GUPROF */ #endif /* I586_CPU || I686_CPU */ + prof_active = 1; cputime_bias = 0; cputime(); } @@ -337,5 +346,32 @@ cputime_clock_pmc_init = FALSE; } #endif + prof_active = 0; +} + +/* XXX Register with sysinit should be a macro in eventhandler.h */ +static void +guprof_tsc_check(void *arg) +{ + + evh_post_tag = EVENTHANDLER_REGISTER(cpufreq_post_change, + tsc_freq_changed, NULL, EVENTHANDLER_PRI_ANY); +} +SYSINIT(guprof_tsc_check, SI_SUB_CONFIGURE, SI_ORDER_ANY, guprof_tsc_check, + NULL); + +/* If the cpu frequency changed while profiling, report a warning. */ +static void +tsc_freq_changed(void *arg, const struct cf_level *level, int status) +{ + static int once; + + /* If there was an error during the transition, don't do anything. */ + if (status != 0) + return; + if (prof_active && cputime_clock == CPUTIME_CLOCK_TSC && !once) { + printf("warning: cpu freq changed while profiling active\n"); + once = 1; + } } #endif /* GUPROF */ --------------010201050001060005050802--