From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id A49EDA046B for ; Fri, 26 Jul 2019 12:15:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 793731C420; Fri, 26 Jul 2019 12:15:19 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 08E481C06A; Fri, 26 Jul 2019 12:15:15 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jul 2019 03:15:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,310,1559545200"; d="scan'208";a="369488027" Received: from aburakov-mobl1.ger.corp.intel.com (HELO [10.252.25.220]) ([10.252.25.220]) by fmsmga005.fm.intel.com with ESMTP; 26 Jul 2019 03:15:13 -0700 To: David Hunt , dev@dpdk.org Cc: stable@dpdk.org References: <20190724131803.30066-1-david.hunt@intel.com> From: "Burakov, Anatoly" Message-ID: Date: Fri, 26 Jul 2019 11:15:12 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190724131803.30066-1-david.hunt@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH v1] examples/power: fix oob frequency oscillations X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 24-Jul-19 2:18 PM, David Hunt wrote: > The branch ratio algorithm in the vm_power_manager sample application > can be very sensitive at patricular loads in a workload, causing > oscillations between min and max frequency. For example, if a > workload is at 50%, scaling up may change the ratio > enough that it immediately thinks it needs to scale down again. > > This patch introduces a sliding window recording the scale up/down > direction for the last 32 samples, and scales up if any samples indicate > we should scale up, otherwise scale down. Each core has it's own window. > > Fixes: 4b1a631b8a8a ("examples/vm_power: add oob monitoring functions") > Cc: stable@dpdk.org > Signed-off-by: David Hunt > --- > examples/vm_power_manager/oob_monitor_x86.c | 34 +++++++++++++++++++-- > examples/vm_power_manager/power_manager.c | 3 +- > examples/vm_power_manager/power_manager.h | 12 ++++++++ > 3 files changed, 44 insertions(+), 5 deletions(-) > > diff --git a/examples/vm_power_manager/oob_monitor_x86.c b/examples/vm_power_manager/oob_monitor_x86.c > index ebd96b205..aecfcb2eb 100644 > --- a/examples/vm_power_manager/oob_monitor_x86.c > +++ b/examples/vm_power_manager/oob_monitor_x86.c > @@ -39,6 +39,7 @@ apply_policy(int core) > int64_t hits_diff, miss_diff; > float ratio; > int ret; > + int freq_window_idx, up_count = 0, i; > > g_active = 0; > ci = get_core_info(); > @@ -101,10 +102,37 @@ apply_policy(int core) > > ratio = (float)miss_diff * (float)100 / (float)hits_diff; > > - if (ratio < ci->branch_ratio_threshold) > - power_manager_scale_core_min(core); > + /* > + * Store the last few directions that the ratio indicates > + * we should take. If there's on 'up', then we scale up One? > + * quickly. If all indicate 'down', only then do we scale > + * down. Each core_details struct has it's own array. > + */ > + freq_window_idx = ci->cd[core].freq_window_idx; > + if (ratio > ci->branch_ratio_threshold) > + ci->cd[core].freq_directions[freq_window_idx] = 1; > else > - power_manager_scale_core_max(core); > + ci->cd[core].freq_directions[freq_window_idx] = 0; > + > + freq_window_idx++; > + freq_window_idx = freq_window_idx & (FREQ_WINDOW_SIZE-1); > + ci->cd[core].freq_window_idx = freq_window_idx; > + > + up_count = 0; > + for (i = 0; i < FREQ_WINDOW_SIZE; i++) > + up_count += ci->cd[core].freq_directions[i]; > + > + if (up_count == 0) { > + if (ci->cd[core].freq_state != FREQ_MIN) { > + power_manager_scale_core_min(core); > + ci->cd[core].freq_state = FREQ_MIN; > + } > + } else { > + if (ci->cd[core].freq_state != FREQ_MAX) { > + power_manager_scale_core_max(core); > + ci->cd[core].freq_state = FREQ_MAX; > + } > + } So it's biased towards scaling up quickly, but it's doing that over a period. Please correct me if i'm wrong as i'm not really familiar with this codebase, but, assuming the window size is long enough, you could be missing opportunities to scale down? For example, if you get a short burst of 1's followed by a long burst of zeroes, you're not scaling down until you go through the entire buffer and overwrite all of the values. I guess that's the point of oscillation prevention, but maybe you could improve the "scale-up" part by only checking a few recent values, rather than the entire buffer? -- Thanks, Anatoly