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

[johansen-osdev at sun.com]

Ah, yeah.  If we don't ever free or reallocate these, then my suggestion
doesn't make a whole lot of sense.  Sorry.

-j

On Wed, Aug 29, 2007 at 12:28:51PM -0700, Bill Holler wrote:
> 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
> >
> 
> _______________________________________________
> intel-platform-discuss mailing list
> intel-platform-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/intel-platform-discuss