Received: by 2002:a25:e7d8:0:0:0:0:0 with SMTP id e207csp766077ybh; Wed, 11 Mar 2020 10:20:50 -0700 (PDT) X-Google-Smtp-Source: ADFU+vuiSKS9ynoFdiAgr1IFoVRiB9EHlAMyQ5PFEqYkiGALKpxffMPI/TpMLFAVKXJgJKNsTmfB X-Received: by 2002:aca:4e08:: with SMTP id c8mr2698155oib.143.1583947250070; Wed, 11 Mar 2020 10:20:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1583947250; cv=none; d=google.com; s=arc-20160816; b=XEdtRu5rbxDjhW+5vaF3Dste66Mb8sbMCtpxp3AkqtTjmG9dFn8hrN9HnpE9Cj03Od pOaQiR0Ah9j9fSXz738Z4iqBwJxJzVUk0yswwqDrhITMvtfiowwpagaHd96U+PCv6Tb5 F/bsOSnaI1JVVG2PBMHJL4IVh2kgJn8FqE133BDKQMTaBk1i++38/QJ7nJy3oG/VmUlN nkGwTRIsKEtaJYy2XYhSZFJKwQC7ZG0wKAo35jbC7u+yeTm0vIqvN+G+pq4vm4pFXvA5 lCIGHHLRfi6q5HggK+q1J1WIiiyBQf5TBT3L2Zbityf2K7p07TZ4Wj0Yzj+ns+fiDFzD IElg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=szcY43oUZmQmror/6kFdB3xisRYuLg0iMZXyeUMQeDo=; b=kMJqwLMM8Ji52gyBCIzbsWO8tfp35UoXQkCc7iAGYMi6vP0p4KrawEbDDFV/hm9VzT hzu1FhKFkmyt4CN1khskszs9w8vXN2t072xJo3hk097T8UAoE3tcQPpCphelxgCyx7AI oab+qLi8T2hhbBs3ff0FBuJeIYy5vqGX8UjgvYxfnheFFRo+T42sDNzbzU0gFdUDsKS2 v8O2j1knFtW0HbBYCTUMNgHk+dfPUEknXtKoKiIH2H80HZUOuuJI8bTSRcqcutPDfEuV XzGPlUqCZn8Zn35dlXsBXvayAOJydQW+vzlnRq3dPWwfVaOPtMmJPTseELqnkne4gQ3I x+yA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z24si1396902otm.188.2020.03.11.10.20.37; Wed, 11 Mar 2020 10:20:50 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730441AbgCKRTV (ORCPT + 99 others); Wed, 11 Mar 2020 13:19:21 -0400 Received: from mga12.intel.com ([192.55.52.136]:51236 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730380AbgCKRTU (ORCPT ); Wed, 11 Mar 2020 13:19:20 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Mar 2020 10:19:20 -0700 X-IronPort-AV: E=Sophos;i="5.70,541,1574150400"; d="scan'208";a="277454902" Received: from rchatre-mobl.amr.corp.intel.com (HELO [10.251.23.31]) ([10.251.23.31]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 11 Mar 2020 10:19:19 -0700 Subject: Re: [PATCH V1 11/13] selftests/resctrl: Change Cache Quality Monitoring (CQM) test To: Sai Praneeth Prakhya , shuah@kernel.org, linux-kselftest@vger.kernel.org Cc: tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, tony.luck@intel.com, babu.moger@amd.com, james.morse@arm.com, ravi.v.shankar@intel.com, fenghua.yu@intel.com, x86@kernel.org, linux-kernel@vger.kernel.org References: <26086dda86f062bba4116878a012a553503924b2.1583657204.git.sai.praneeth.prakhya@intel.com> From: Reinette Chatre Message-ID: Date: Wed, 11 Mar 2020 10:19:18 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sai, On 3/10/2020 7:46 PM, Sai Praneeth Prakhya wrote: > On Tue, 2020-03-10 at 15:18 -0700, Reinette Chatre wrote: >> On 3/6/2020 7:40 PM, Sai Praneeth Prakhya wrote: >>> .mum_resctrlfs = 0, >>> .filename = RESULT_FILE_NAME, >>> - .mask = ~(long_mask << n) & long_mask, >>> - .span = cache_size * n / count_of_bits, >>> .num_of_runs = 0, >>> - .setup = cqm_setup, >>> + .setup = cqm_setup >>> }; >>> + int ret; >>> + char schemata[64]; >>> + unsigned long long_mask; >>> >>> - if (strcmp(benchmark_cmd[0], "fill_buf") == 0) >>> - sprintf(benchmark_cmd[1], "%lu", param.span); >>> + ret = remount_resctrlfs(1); >>> + if (ret) >>> + return ret; >> >> Here resctrl is remounted and followed by some changes to the root >> group's schemata. That is followed by a call to resctrl_val that >> attempts to remount resctrl again that will undo all the configurations >> inbetween. > > No, it wouldn't because mum_resctrlfs is 0. When resctrl FS is already mounted > and mum_resctrlfs is 0, then remount_resctrlfs() is a noop. > I missed that. Thank you. fyi ... when I tried these tests I encountered the following error related to unmounting: [SNIP] ok Write schema "L3:1=7fff" to resctrl FS ok Write schema "L3:1=ffff" to resctrl FS ok Write schema "L3:1=1ffff" to resctrl FS ok Write schema "L3:1=3ffff" to resctrl FS # Unable to umount resctrl: Device or resource busy # Results are displayed in (Bytes) ok CQM: diff within 5% for mask 1 # alloc_llc_cache_size: 2883584 # avg_llc_occu_resc: 2973696 ok CQM: diff within 5% for mask 3 [SNIP] This seems to originate from resctrl_val() that forces an unmount but if that fails the error is not propagated. >>> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c >>> b/tools/testing/selftests/resctrl/resctrl_val.c >>> index 271cb5c976f5..c59fad6cb9b0 100644 >>> --- a/tools/testing/selftests/resctrl/resctrl_val.c >>> +++ b/tools/testing/selftests/resctrl/resctrl_val.c >>> @@ -705,29 +705,21 @@ int resctrl_val(char **benchmark_cmd, struct >>> resctrl_val_param *param) >>> goto out; >>> } >>> >>> - /* Give benchmark enough time to fully run */ >>> - sleep(1); >>> - >>> /* Test runs until the callback setup() tells the test to stop. */ >>> while (1) { >>> + ret = param->setup(param); >>> + if (ret) { >>> + ret = 0; >>> + break; >>> + } >>> + >>> + /* Measure vals sleeps for a second */ >>> if ((strcmp(resctrl_val, "mbm") == 0) || >>> (strcmp(resctrl_val, "mba") == 0)) { >>> - ret = param->setup(param); >>> - if (ret) { >>> - ret = 0; >>> - break; >>> - } >>> - (I refer to the above snippet in my comment below) >>> ret = measure_vals(param, &bw_resc_start); >>> if (ret) >>> break; >>> } else if (strcmp(resctrl_val, "cqm") == 0) { >>> - ret = param->setup(param); >>> - if (ret) { >>> - ret = 0; >>> - break; >>> - } >>> - sleep(1); >>> ret = measure_cache_vals(param, bm_pid); >>> if (ret) >>> break; >> >> This change affects not just the cache monitoring test. Could this >> change be extracted into its own patch to be clear what is done here and >> how it impacts the other tests? > > This change shouldn't impact other tests (i.e. CAT) because CAT will not call > resctrl_val(). I was referring to the snippet above that seems to impact the "mbm" and "mba" tests by moving the call to "param->setup" for the them. > >>> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c >>> b/tools/testing/selftests/resctrl/resctrlfs.c >>> index 52452bb0178a..bd81a13ff9df 100644 >>> --- a/tools/testing/selftests/resctrl/resctrlfs.c >>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c >>> @@ -365,11 +365,7 @@ void run_benchmark(int signum, siginfo_t *info, void >>> *ucontext) >>> memflush = atoi(benchmark_cmd[3]); >>> operation = atoi(benchmark_cmd[4]); >>> sprintf(resctrl_val, "%s", benchmark_cmd[5]); >>> - >>> - if (strcmp(resctrl_val, "cqm") != 0) >>> - buffer_span = span * MB; >>> - else >>> - buffer_span = span; >>> + buffer_span = span * MB; >> >> This change seems to change the buffer_span used by the other tests. It >> is not obvious why this change is made to other tests while this commit >> intends to focus on the cache monitoring test. Perhaps this can be split >> into a separate patch to make this clear? > Got it. Thank you. Reinette