[intel-platform-discuss] Code review for several MONITOR/MWAIT idle loop CRs

[Bill Holler]

The original implementation (pre-ON-putback) used a kmem_cache
which took care of the alignment issues as you mentioned.   :-)
We changed the implementation to use kmem_alloc() during the
original review due to the memory overhead as Joe noted.

Thank you,
Bill


Joe Bonasera wrote:
>
> Creating a kmem cache for these would waste lots of memory.
> Distinct caches have lots of overhead, especially given we
> allocate just one of these per processor and don't ever free
> or re-allocate them.
>
> johansen-osdev at sun.com wrote:
>> Bill,
>> These changes generally quite good.  The only thing that I would
>> consider changing would be the use of kmem_alloc() and size_actual,
>> buf_actual in the struct mwait_info.
>>
>> If you instead use kmem_cache_alloc() for these structures, you can
>> specify the required alignment when you create the cache as part of
>> kmem_cache_create().  This assumes that all mwait_info structs will need
>> to be aligned to the same boundaries.  (I don't know if that's a safe
>> assumption in this case.)
>>
>> Hope that helps,
>>
>> -j
>>
>>
>> On Tue, Aug 28, 2007 at 04:14:45PM -0700, Bill Holler wrote:
>>> http://cr.opensolaris.org/~bholler/mwait_fixes
>>>
>>> Here is a webrev code review for the following idle loop MONITOR/MWAIT
>>> CRs (Change Requests):
>>>
>>> 6577948 <http://monaco.sfbay.sun.com/detail.jsp?cr=6577948> 
>>> mach_alloc_mwait leaks memory when a CPU fails to start
>>> 6588054 <http://monaco.sfbay.sun.com/detail.jsp?cr=6588054> panic() 
>>> in mach_alloc_mwait() should be changed to degraded operation...
>>> 6596141 <http://monaco.sfbay.sun.com/detail.jsp?cr=6596141> Solaris 
>>> should not use an unmodified MWAIT idle loop on AMD 10h due to 
>>> increased power consumption
>>>
>>>
>>> While these are relatively low priority, I want to get these in soon 
>>> for
>>> maximum S10U5 soak time.
>>>
>>> Testing completed:
>>> 1) The x86 build of this kernel has passed ON PIT DIY.  The SPARC build
>>>      is currently in ON PIT DIY and looks ok.  (There are no common 
>>> changes
>>>      that should effect SPARC.)
>>> 2) libmicro did not regress.
>>>     PERF PIT is not planned as these are enable/disable changes 
>>> which should
>>>     not change performance in either state.
>>> 3) The memory leak is fiked in unit testing with all non-boot cpus 
>>> forced to
>>>     fail to online using cpufailset debug hook.
>>> 4) My desktop has been running this for 2 weeks without incident.
>>>
>>> Ongoing testing:
>>> 1) Barcelona testing is ongoing.
>>> 2) 6588054 error injection testing is ongoing.
>>>
>>> Thank you,
>>> Bill Holler
>>>
>>> _______________________________________________
>>> intel-platform-discuss mailing list
>>> intel-platform-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/intel-platform-discuss
>> _______________________________________________
>> intel-platform-discuss mailing list
>> intel-platform-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/intel-platform-discuss
>