From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: Calculating real cpu usage of Xen domains correctly! Date: Thu, 24 Feb 2005 14:43:24 -0600 Message-ID: <421E3C6C.2070507@us.ibm.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit In-Reply-To: Sender: xen-devel-admin@lists.sourceforge.net Errors-To: xen-devel-admin@lists.sourceforge.net List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , List-Archive: To: John L Griffin Cc: xen-devel@lists.sourceforge.net List-Id: xen-devel@lists.xenproject.org Hi John, John L Griffin wrote: >My quick perusal of the current xeno-unstable code suggests that this >SCHED_OP call is a null call. The SCHED_OP macro attempts to call the >"do_block" function pointed to in the "struct scheduler sched_bvt_def" >array, but this function pointer is never initialized, so it just does a >NOP. > > Yes. I agree. >It appears that what your patch does is limit when cpu_time gets updated, >such that the time only gets updated when the exec_domain is in the >BLOCKED state: > > No. Currently, cpu_time will always get updated for the prev domain when __enter_scheduler() is called. My patch modifies that behavior the behavior so that it only gets updated if prev->blocked is false or if prev->blocked is true but there are events pending for the domain. The last part may not be right. > if ( test_bit(EDF_BLOCKED, &prev->ed_flags) ) > prev->cpu_time += now - prev->lastschd; > } > > Sorry, this is my fault, my mail client badly munged the patch when I copy-pasted it. Let me show you the code: if ( test_bit(EDF_BLOCKED, &prev->ed_flags) ) { /* This check is needed to avoid a race condition. */ if ( event_pending(prev) ) clear_bit(EDF_BLOCKED, &prev->ed_flags); else SCHED_OP(do_block, prev); } prev->cpu_time += now - prev->lastschd; Was changed to: if ( test_bit(EDF_BLOCKED, &prev->ed_flags) ) { /* This check is needed to avoid a race condition. */ if ( event_pending(prev) ) { clear_bit(EDF_BLOCKED, &prev->ed_flags); prev->cpu_time += now - prev->lastschd; } else SCHED_OP(do_block, prev); } else { prev->cpu_time += now - prev->lastschd; } It's not pretty at all. I'm not sure if the update after clear_bit() is necessary either. >What this seems to be saying (in regard to your patch working) is that the >cpu_time is updated when the domain relinquishes the CPU by block()ing, >but cpu_time doesn't get updated if it relinquishes the CPU by yield()ing. > > No, it's the opposite. Sorry, I think the whitespace munging made the patch confusing. >I wonder why this works. Is anyone familiar with block() vs yield(), that >could lend some insight into what's going on? > > I'm quite confident this is the cause of the problem. I'm just not sure about that one cpu_time update. Regards, Anthony Liguori >JLG > > > > ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click