Date: Tue, 17 Jul 2012 16:39:31 -0400 From: Steve McCoy <smccoy@greatbaysoftware.com> To: freebsd-stable@freebsd.org Cc: Charles Owens <cowens@greatbaysoftware.com>, John Baldwin <jhb@freebsd.org> Subject: Re: mfi(4) IO performance regression, post 8.1 Message-ID: <5005CD83.306@greatbaysoftware.com> In-Reply-To: <201207130939.54311.jhb@freebsd.org> References: <4FDABA0B.5030702@greatbaysoftware.com> <4FFF34BA.9030002@greatbaysoftware.com> <4FFF9A50.40006@greatbaysoftware.com> <201207130939.54311.jhb@freebsd.org>
index | next in thread | previous in thread | raw e-mail
On 7/13/12 9:39 AM, John Baldwin wrote:
> On Thursday, July 12, 2012 11:47:28 pm Steve McCoy wrote:
>> On 7/12/12 4:34 PM, Steve McCoy wrote:
>>>>> John Baldwin wrote:
>>>>
>>>> Barring that, can you do a binary search of kernels from stable/8
>>>> between 8.1
>>>> and 8.2 on an 8.1 world to see which commit caused the change in write
>>>> performance?
>>>>
>>>
>>> Hi John, I'm working with Charles to narrow this down.
>>>
>>> Looks like revision 212229 is the culprit, or at least around the same
>>> time to it, if this change isn't what slowed things down. The change to
>>> sys/kern/vfs_bio.c modifies some synchronization in dev_strategy():
>>>
>>
>> Actually, hold that thought. I had a hunch that I wasn't thorough
>> enough, so I decided to try 212228 — the performance is the same as with
>> 212229, so vfs_bio seems to be out of the picture. I'm going to binary
>> search between 209459 and 212229, and see what I find.
>
> Ok. Please let me know what you find. Thanks!
>
Alright, I've finally narrowed it down to r209897, which only affects
acpi_cpu_idle():
--- stable/8/sys/dev/acpica/acpi_cpu.c 2010/06/23 17:04:42 209471
+++ stable/8/sys/dev/acpica/acpi_cpu.c 2010/07/11 11:58:46 209897
@@ -930,12 +930,16 @@
/*
* Execute HLT (or equivalent) and wait for an interrupt. We can't
- * calculate the time spent in C1 since the place we wake up is an
- * ISR. Assume we slept half of quantum and return.
+ * precisely calculate the time spent in C1 since the place we wake up
+ * is an ISR. Assume we slept no more then half of quantum.
*/
if (cx_next->type == ACPI_STATE_C1) {
- sc->cpu_prev_sleep = (sc->cpu_prev_sleep * 3 + 500000 / hz) / 4;
+ AcpiHwRead(&start_time, &AcpiGbl_FADT.XPmTimerBlock);
acpi_cpu_c1();
+ AcpiHwRead(&end_time, &AcpiGbl_FADT.XPmTimerBlock);
+ end_time = acpi_TimerDelta(end_time, start_time);
+ sc->cpu_prev_sleep = (sc->cpu_prev_sleep * 3 +
+ min(PM_USEC(end_time), 500000 / hz)) / 4;
return;
}
My current guess is that AcpiHwRead() is a problem on our hardware. It's
an isolated change and, to my desperate eyes, the commit message implies
that it isn't critical — Do you think we could buy ourselves some time
by pulling it out of our version of the kernel? Or is this essential for
correctness? Any thoughts are appreciated, thanks!
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5005CD83.306>