Hello Liu Jiang,
Here are some quick comments:
General: There should be an Intel Copyright added to files
you are significantly changing (IIRC per the Intel/SUN
agreement). New Intel Copyrights are missing.
intr.c line 941:
Does a callback in do_interrupt() add too much latency?
I assume this is here only to catch idle-wakeup from the
C1 idle loops which enable interrupt when waking up
from the HLT or MWAIT instruction?
General: The Deep C-state idle loop only uses C1 when it
predicts added latency will hurt performance, or the CPU
will not be idle long enough to enter a deeper C-state.
Should this knowledge be used in cpu_idle_enter() and
cpu_idle_exit()?
General: How do libmicro numbers look?
cpu.h line 22: update Copyright date to 2009. ;-)
cpu_idle.h line 37 - 40 and cpu.h lines 79:
The CPU_ACPI_C# values are defined by ACPI. There
could be a comment in cpu.h above line 79 mentioning
that these IDLE_STATE_# defines must match the ACPI
definition. Otherwise maintainers may not know where
these numbers came from.
I have only very quickly looked at the cpu_event files, and
still need to review these.
cpu_event.c line 453: s/unregisger/unregister/
On 03/31/09 20:35, Liu, Jiang wrote:
> Hi buddies,
> We are planning to integrate cpu idle notification into b113 if thing
> goes smoothly, so quick response is appreciated.
> Thanks!
>
> Liu, Jiang <> wrote:
>
>> Hi buddies,
>> I'm glad to announce the first release of PSARC/2009/115 CPU idle
>> notification implementation. Great thanks to Randy and Garrett for
>> supporting
>> this project.
>> The gate for CPU idle notification and FIPE project is
>> ssh://hg.opensolaris.org/tesla/mempm-fipe.
>> The webrev release is at
>> http://cr.opensolaris.org/~gerry/cpuidle_20090327/. The PSARC
>> onepager is at
>> http://cr.opensolaris.org/~gerry/CPU_idle_notification_onepager.txt.
>> Could you please help to review the source code and give us any
>> comments and suggestions about it? There are known limitations in
>> current implementation
>> and several directions for future work as below:
>> 1) Currently implementation only supports x86, supporting of SPARC
>> will be added later (I'm not familiar with SPARC and needs help from
>> SPARC experts.)
>>
It is good start having cpu_event.c in common code.
>> 2) There's unit test code in current code, which will be removed in
>> final release.
>> 3) Needs your comments about moving "tlb_going_idle/tlb_service"
>> and "cpu_dtrace_idle_probe" into CPU idle notifcation as callbacks. Then
>> we could control relative calling order with other idle callbacks. It will
>> also
>> help to remove unnecessary calling of "tlb_service" in do_interrupt.
>>
IIRC the HLT instruction requires interrupts be unmasked.
As far as I know it is not possible to ensure cpu_idle_exit()
runs before interrupts in cpu_idle()? Some of the earlier
MWAIT implementations did not allow interrupts to be
masked either. These will prevent tlb_service() from being
moved out of do_interrupt().
It seems reasonable to move cpu_dtrace_idle_probe into
CPU idle notification callback.
>> 4) Needs your comments about better cooperation with CPU deep C
>> driver to avoid duplicated work both in cpupm and idle notification.
>>
I will look at this more.
Unfortunately Deep C-states are not enabled on many platforms.
That prevents CPU Idle Callback from relying too heavily on
the Deep C-state code and collected data. :-(
At first glance cpupm_wakeup_cstate_data() can be a CPU Idle
Callback without any additional work.
>> 5) Needs your comments on whether need to add "check_wakeup"
>> hook to cpu_idle_enter, then cpu_idle_enter could detect CPU wakeup as soon
>> as possible.
>>
Can you describe "check_wakeup" a little more?
I do not understand what this would do?
Regards,
Bill
>> 6) There are several violations to PSARC/2009/115.
>> So please share your ideas/suggestions with me!
>> Thanks!
>>
>> Liu Jiang (Gerry)
>> Senior Software Engineer
>> OpenSolaris, OTC, SSG, Intel
>> Tel: (8610)82171643
>> iNet: 8-758-1643
>> Location: Raycom 9W013
>>
>>
>> _______________________________________________
>> tesla-dev mailing list
>> tesla-dev at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>>
>
> Liu Jiang (Gerry)
> OpenSolaris, OTC, SSG, Intel
> _______________________________________________
> tesla-dev mailing list
> tesla-dev at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/tesla-dev
>
