Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 9 Jun 2012 23:31:34 +0300
From:      Theodor-Iulian Ciobanu <thciobanu@nth.ro>
To:        freebsd-stable@freebsd.org
Subject:   Re: [stable 9] broken hwpstate calls
Message-ID:  <20120609233134.0000711b@unknown>
In-Reply-To: <4FCFE178.9080505@FreeBSD.org>
References:  <1337319129.2915.4.camel@powernoodle-l7> <4FB6765A.2050307@FreeBSD.org> <1337710214.2916.8.camel@powernoodle-l7.corp.yahoo.com> <20120525163653.b61a08e2.lists@yamagi.org> <4FBFA9A9.7020806@FreeBSD.org> <4FBFBD39.7000105@FreeBSD.org> <4FBFDFFB.9020501@FreeBSD.org> <4FBFE624.1020208@FreeBSD.org> <20120526090233.f638c1d2.lists@yamagi.org> <4FC0A3A1.80200@FreeBSD.org> <4FC7D464.20602@FreeBSD.org> <4FCFD2A1.60706@FreeBSD.org> <4FCFE178.9080505@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 06 Jun 2012 19:02:16 -0400
Jung-uk Kim <jkim@FreeBSD.org> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 2012-06-06 17:58:57 -0400, Andriy Gapon wrote:
> > on 31/05/2012 23:28 Jung-uk Kim said the following:
> >> It is simple but I don't like locking scheduler, binding CPU, and
> >> writing the same MSR, multiple times for each core.
> > 
> > Not sure if parse this.  The MSR is _written_ /once/ for each
> > core. (BTW, "locking scheduler" is not a completely accurate
> > description of what thread_lock does)
> 
> I apologize.  I didn't see the whole picture and read your patch
> wrong.
> 
> Any way, hwpstate still isn't quite right even without your patch.
> 
> sys/kern/kern_cpu.c
> 	cpufreq_curr_sysctl() ->
> 	CPUFREQ_SET() ->	/* for all CPU devices */
> 	cf_set_method() ->	/* thread_lock(), sched_bind(), ...
> */ CPUFREQ_DRV_SET() ->
> sys/x86/cpufreq/hwpstate.c
> 	hwpstate_set() ->
> 	hwpstate_goto_pstate()	/* for each CPU unit */
> 				/* thread_lock(), sched_bind(), ... */
> 
> Therefore, "sysctl dev.cpu.0.cpufreq=<freq>" loops n^2 times (i.e., n
> times per CPU) where n is number of CPUs.  At least, it should check
> unit == 0, e.g.,
> 
> hwpstate_goto_pstate(...)
> {
> ...
> 	if (unit == 0) {
> 		/* XXX Is this really necessary? */
> 		CPU_FOREACH(i) {
> 			...
> 			wrmsr(MSR_AMD_10H_11H_CONTROL, id);
> 			...
> 		}
> 	}
> 	/* Check the current P-state. */
> 	for (...) {
> 		...
> 		msr = rdmsr(MSR_AMD_10H_11H_STATUS);
> 		if (msr == id)
> 			break;
> 		...
> 	}
> 	/* XXX Maybe your patch here? */
> ...
> }
> 
> >> Besides, it introduces more delay and you may be reading the
> >> correct status because of that. :-P
> > 
> > Having a separate reading pass does introduce more delay indeed. 
> > Reading the correct status is a good thing, OTOH.
> 
> That's what I said.
> 
> > Why would anyone want to read incorrect status?  (just want to note
> > that "correct" and "expected" are different things)
> 
> Okay, okay.
> 
> >> If people really think checking MSRC001_0071[18:16] is unworthy
> >> for
> > 
> > Well, "other people" hasn't demonstrated/proved/convinced yet that
> > it is worthy
> > 
> >> Bulldozer, I prefer skipping status check
> > 
> > That's what I suggested from the very start.
> 
> Buy me a Bulldozer and I'll fix it for you! :-P

If it's of any help, I have an Opteron 6274 I'd be willing to test some
patches on, to get Turbo Core working.

> >> but I disagree with this patch.
> > 
> > Since I am not invested in this issue (I am not affected by the
> > problem and I do not have any personal attachment to the code in
> > question), I will just defer any decision to those who do care
> > about the problem.  I hope that a fix will be provided in the end.
> 
> Same here.
> 
> Jung-uk Kim
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (FreeBSD)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
> 
> iEYEARECAAYFAk/P4XgACgkQmlay1b9qnVP8cgCgl9sAzyE956YjB2B3bK0wvOHu
> n64Anih7sdWYQgflQVHuUGstdk05Fs9i
> =2dS0
> -----END PGP SIGNATURE-----
> _______________________________________________
> freebsd-stable@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to
> "freebsd-stable-unsubscribe@freebsd.org"

-- 
Theo



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120609233134.0000711b>