Message ID | 20230713131932.133258-5-ilpo.jarvinen@linux.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a6b2:0:b0:3e4:2afc:c1 with SMTP id c18csp1821748vqm; Thu, 13 Jul 2023 06:23:27 -0700 (PDT) X-Google-Smtp-Source: APBJJlF+U0ePdev5m6NjvPk/FDL0REjpzLGywZA2AGh6e05ftyoaXVZh8ealhKr+oeoLbPlF5WIW X-Received: by 2002:a17:906:738c:b0:993:f2b4:13c9 with SMTP id f12-20020a170906738c00b00993f2b413c9mr1511692ejl.21.1689254607672; Thu, 13 Jul 2023 06:23:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1689254607; cv=none; d=google.com; s=arc-20160816; b=V3+CV4IHQ6dIwDiRDisXmUc3aQ82T634cIs45Y+fqS3kqDsvrEmvPXz89MEhrsBk/n 9o58Nr9x4YJ4OGEJW+a268/cgKxS/ZEU7hWz1epWx5pcVKnwTiKD7QHKdQ+HfQhTjDVw j7TNt3p3oc2qsXV7Od300Nurs1EHeZEz3pfZXpOEud9hOKh9ETNzS02m0RrnClEvNd1+ ZKQsVaFBpx/s8eTC/F3N/dqDdplF20dj/AQoVZ56EvU9iP1yXBREWibhbfGhA1NANcA+ qKwhhqAtkeMBowSUG7uPJyrEoy6CkyGNq1G39g4EfjKAZsShpO/kB28Bl3TyHspfazLk lfcw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=0LLUpqGHRxNZQm9k99I7o0ydSvwm9+uS/tKGualXDnw=; fh=zadu2NYU0jRL4pf+hGpYnN61BXmsILopbhQv8OVWGQI=; b=jXWDUjKpZJJLzt/jBk5ezmGWyc4A2ZFWOzAStB4KjXv2PQUUE9JbjYsrUjjegflgIZ 2tYkNbCxDTsw9tW3PXHGFgasPE+05POKGD6dNlNFFkfPvJa+ey9uoWLxTrEVLK6QPiJ/ vgruQRT1N9wlIHCLJOr3WlJZ1SSugqIWdmAawFf+eyXtu1DckcdSkYK1eYCmI2Mr3exV ynW2sGdVhW/hcKbRAIHc3Fp9OVnngo70FEnaVsN84XMBHayrir9LLe3Lug/CC8HxMxnV SMluI8EvmSdJIZn6y/Z1M0jb6jQEjaQ6W0/9nwSypSzl6JKzxinOQSqiHFEXPih/T6fR kfPQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=hO1C6B0l; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x20-20020a170906711400b0098df1cbe2acsi7177295ejj.997.2023.07.13.06.23.03; Thu, 13 Jul 2023 06:23:27 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=hO1C6B0l; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234846AbjGMNVG (ORCPT <rfc822;ybw1215001957@gmail.com> + 99 others); Thu, 13 Jul 2023 09:21:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52124 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230514AbjGMNUl (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 13 Jul 2023 09:20:41 -0400 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 72DC1213B; Thu, 13 Jul 2023 06:20:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1689254431; x=1720790431; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=SfPvkmzWJcJdc6LqGPirAEzeqOxBh6wxev3IG7Dp8Fo=; b=hO1C6B0lLC21mJUarCQLpq/Uts2FxsxdaUfGgLbXibQdG8ciS/IlZzfV XXprmbV2RzWpSy2EoBhloYz440l/XnqnnUVB4IHesSF79IlxpCm7j1IKg hVG3WtxKowpmtpuvoxxBlNj7U2i60Z+zC8yCj77jN/WXxRw4e1yJXARD5 vC5SsCYpxYDHLWdQIu1gmYkIxC08W0TyUJycymX8kQJo0DEGTbUkraBu0 ACvcLo2xwkZWf9lcyYy/WmnVhr5HTA7SXbbMia/iMKtZgCwurFZZ5183n BZjP2VII5LLHHiH/WnS5jNxrwZJnxR/fr0yo/Pp0ZWcB45wrCLSlB2bS+ g==; X-IronPort-AV: E=McAfee;i="6600,9927,10770"; a="345496593" X-IronPort-AV: E=Sophos;i="6.01,202,1684825200"; d="scan'208";a="345496593" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jul 2023 06:20:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10770"; a="968615770" X-IronPort-AV: E=Sophos;i="6.01,202,1684825200"; d="scan'208";a="968615770" Received: from ijarvine-mobl2.ger.corp.intel.com ([10.251.222.39]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Jul 2023 06:20:28 -0700 From: =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> To: linux-kselftest@vger.kernel.org, Reinette Chatre <reinette.chatre@intel.com>, Shuah Khan <shuah@kernel.org>, Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>, Fenghua Yu <fenghua.yu@intel.com>, Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>, Babu Moger <babu.moger@amd.com>, linux-kernel@vger.kernel.org Cc: =?utf-8?q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com> Subject: [PATCH v4 04/19] selftests/resctrl: Close perf value read fd on errors Date: Thu, 13 Jul 2023 16:19:17 +0300 Message-Id: <20230713131932.133258-5-ilpo.jarvinen@linux.intel.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20230713131932.133258-1-ilpo.jarvinen@linux.intel.com> References: <20230713131932.133258-1-ilpo.jarvinen@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1771311839463682467 X-GMAIL-MSGID: 1771311839463682467 |
Series |
selftests/resctrl: Fixes and cleanups
|
|
Commit Message
Ilpo Järvinen
July 13, 2023, 1:19 p.m. UTC
Perf event fd (fd_lm) is not closed on some error paths.
Always close fd_lm in get_llc_perf() and add close into an error
handling block in cat_val().
Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
tools/testing/selftests/resctrl/cache.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Comments
Hi Ilpo, On 7/13/2023 6:19 AM, Ilpo Järvinen wrote: > Perf event fd (fd_lm) is not closed on some error paths. > > Always close fd_lm in get_llc_perf() and add close into an error > handling block in cat_val(). > > Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest") > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > tools/testing/selftests/resctrl/cache.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c > index 8a4fe8693be6..ced47b445d1e 100644 > --- a/tools/testing/selftests/resctrl/cache.c > +++ b/tools/testing/selftests/resctrl/cache.c > @@ -87,21 +87,20 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no) > static int get_llc_perf(unsigned long *llc_perf_miss) > { > __u64 total_misses; > + int ret; > > /* Stop counters after one span to get miss rate */ > > ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); > > - if (read(fd_lm, &rf_cqm, sizeof(struct read_format)) == -1) { > + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); > + close(fd_lm); > + if (ret == -1) { > perror("Could not get llc misses through perf"); > - > return -1; > } > > total_misses = rf_cqm.values[0].value; > - > - close(fd_lm); > - > *llc_perf_miss = total_misses; > > return 0; > @@ -253,6 +252,7 @@ int cat_val(struct resctrl_val_param *param) > memflush, operation, resctrl_val)) { > fprintf(stderr, "Error-running fill buffer\n"); > ret = -1; > + close(fd_lm); > break; > } > Instead of fixing these existing patterns I think it would make the code easier to understand and maintain if it is made symmetrical. Having the perf event fd opened in one place but its close() scattered elsewhere has the potential for confusion and making later mistakes easy to miss. What if perf event fd is closed in a new "disable_llc_perf()" that is matched with "reset_enable_llc_perf()" and called from cat_val()? I think this raises another issue with the test trickery where measure_cache_vals() has some assumptions about state based on the test name. Reinette
On Thu, 13 Jul 2023, Reinette Chatre wrote: > Hi Ilpo, > > On 7/13/2023 6:19 AM, Ilpo Järvinen wrote: > > Perf event fd (fd_lm) is not closed on some error paths. > > > > Always close fd_lm in get_llc_perf() and add close into an error > > handling block in cat_val(). > > > > Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest") > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > --- > > tools/testing/selftests/resctrl/cache.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c > > index 8a4fe8693be6..ced47b445d1e 100644 > > --- a/tools/testing/selftests/resctrl/cache.c > > +++ b/tools/testing/selftests/resctrl/cache.c > > @@ -87,21 +87,20 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no) > > static int get_llc_perf(unsigned long *llc_perf_miss) > > { > > __u64 total_misses; > > + int ret; > > > > /* Stop counters after one span to get miss rate */ > > > > ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); > > > > - if (read(fd_lm, &rf_cqm, sizeof(struct read_format)) == -1) { > > + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); > > + close(fd_lm); > > + if (ret == -1) { > > perror("Could not get llc misses through perf"); > > - > > return -1; > > } > > > > total_misses = rf_cqm.values[0].value; > > - > > - close(fd_lm); > > - > > *llc_perf_miss = total_misses; > > > > return 0; > > @@ -253,6 +252,7 @@ int cat_val(struct resctrl_val_param *param) > > memflush, operation, resctrl_val)) { > > fprintf(stderr, "Error-running fill buffer\n"); > > ret = -1; > > + close(fd_lm); > > break; > > } > > > > Instead of fixing these existing patterns I think it would make the code > easier to understand and maintain if it is made symmetrical. > Having the perf event fd opened in one place but its close() > scattered elsewhere has the potential for confusion and making later > mistakes easy to miss. > > What if perf event fd is closed in a new "disable_llc_perf()" that > is matched with "reset_enable_llc_perf()" and called > from cat_val()? > > I think this raises another issue with the test trickery where > measure_cache_vals() has some assumptions about state based on the > test name. I very much agree on the principle here, and thus I already have created patches which will do a major cleanup on this area. The cleaned-up code has pe_fd local var to cat_val() and handles closing it in cat_val() with the usual patterns. However, the patch is currently resides post L3 CAT test rewrite. Backporting the cleanups/refactors into this series would require considerable effort due to how convoluted all those n-step cleanup patches and L3 CAT test rewrite are in this area. There's just very much to cleanup here and L3 rewrite will touch the same areas so its a net full of conflicts. Do you want me to spend the effort to backport them into this series (I expect will take some time)? I currently have these items pending besides this series (in order): - L3 CAT test rewrite and its preparatory patches - More cleanups (including the pe_fd cleanup) - New generalized test framework - L2 CAT test
Hi Ilpo, On 7/14/2023 3:35 AM, Ilpo Järvinen wrote: > On Thu, 13 Jul 2023, Reinette Chatre wrote: >> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote: >>> Perf event fd (fd_lm) is not closed on some error paths. >>> >>> Always close fd_lm in get_llc_perf() and add close into an error >>> handling block in cat_val(). >>> >>> Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest") >>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> >>> --- >>> tools/testing/selftests/resctrl/cache.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c >>> index 8a4fe8693be6..ced47b445d1e 100644 >>> --- a/tools/testing/selftests/resctrl/cache.c >>> +++ b/tools/testing/selftests/resctrl/cache.c >>> @@ -87,21 +87,20 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no) >>> static int get_llc_perf(unsigned long *llc_perf_miss) >>> { >>> __u64 total_misses; >>> + int ret; >>> >>> /* Stop counters after one span to get miss rate */ >>> >>> ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); >>> >>> - if (read(fd_lm, &rf_cqm, sizeof(struct read_format)) == -1) { >>> + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); >>> + close(fd_lm); >>> + if (ret == -1) { >>> perror("Could not get llc misses through perf"); >>> - >>> return -1; >>> } >>> >>> total_misses = rf_cqm.values[0].value; >>> - >>> - close(fd_lm); >>> - >>> *llc_perf_miss = total_misses; >>> >>> return 0; >>> @@ -253,6 +252,7 @@ int cat_val(struct resctrl_val_param *param) >>> memflush, operation, resctrl_val)) { >>> fprintf(stderr, "Error-running fill buffer\n"); >>> ret = -1; >>> + close(fd_lm); >>> break; >>> } >>> >> >> Instead of fixing these existing patterns I think it would make the code >> easier to understand and maintain if it is made symmetrical. >> Having the perf event fd opened in one place but its close() >> scattered elsewhere has the potential for confusion and making later >> mistakes easy to miss. >> >> What if perf event fd is closed in a new "disable_llc_perf()" that >> is matched with "reset_enable_llc_perf()" and called >> from cat_val()? >> >> I think this raises another issue with the test trickery where >> measure_cache_vals() has some assumptions about state based on the >> test name. > > I very much agree on the principle here, and thus I already have created > patches which will do a major cleanup on this area. The cleaned-up code > has pe_fd local var to cat_val() and handles closing it in cat_val() with > the usual patterns. > > However, the patch is currently resides post L3 CAT test rewrite. > Backporting the cleanups/refactors into this series would require > considerable effort due to how convoluted all those n-step cleanup patches > and L3 CAT test rewrite are in this area. There's just very much to > cleanup here and L3 rewrite will touch the same areas so its a net > full of conflicts. > > Do you want me to spend the effort to backport them into this series > (I expect will take some time)? Considering the "Fixes" tag, having a smaller fix that can easily be backported would be ideal so I am ok with deferring a bigger rework. I do think this fix can be made more robust with a couple of small changes that should not introduce significant conflicts: * initialize fd_lm to -1 * do not close() fd_lm in get_llc_perf() but instead move its close() to at exit of cat_val(). * add check in get_llc_perf() that it does not attempt ioctl() on "fd_lm == -1" (later addition would be error checking of the ioctl()) > I currently have these items pending besides this series (in order): > - L3 CAT test rewrite and its preparatory patches > - More cleanups (including the pe_fd cleanup) > - New generalized test framework > - L2 CAT test Thank you very much for taking this on. Reinette
On Fri, 14 Jul 2023, Reinette Chatre wrote: > On 7/14/2023 3:35 AM, Ilpo Järvinen wrote: > > On Thu, 13 Jul 2023, Reinette Chatre wrote: > >> On 7/13/2023 6:19 AM, Ilpo Järvinen wrote: > >>> Perf event fd (fd_lm) is not closed on some error paths. > >>> > >>> Always close fd_lm in get_llc_perf() and add close into an error > >>> handling block in cat_val(). > >>> > >>> Fixes: 790bf585b0ee ("selftests/resctrl: Add Cache Allocation Technology (CAT) selftest") > >>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > >>> --- > >>> tools/testing/selftests/resctrl/cache.c | 10 +++++----- > >>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c > >>> index 8a4fe8693be6..ced47b445d1e 100644 > >>> --- a/tools/testing/selftests/resctrl/cache.c > >>> +++ b/tools/testing/selftests/resctrl/cache.c > >>> @@ -87,21 +87,20 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no) > >>> static int get_llc_perf(unsigned long *llc_perf_miss) > >>> { > >>> __u64 total_misses; > >>> + int ret; > >>> > >>> /* Stop counters after one span to get miss rate */ > >>> > >>> ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); > >>> > >>> - if (read(fd_lm, &rf_cqm, sizeof(struct read_format)) == -1) { > >>> + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); > >>> + close(fd_lm); > >>> + if (ret == -1) { > >>> perror("Could not get llc misses through perf"); > >>> - > >>> return -1; > >>> } > >>> > >>> total_misses = rf_cqm.values[0].value; > >>> - > >>> - close(fd_lm); > >>> - > >>> *llc_perf_miss = total_misses; > >>> > >>> return 0; > >>> @@ -253,6 +252,7 @@ int cat_val(struct resctrl_val_param *param) > >>> memflush, operation, resctrl_val)) { > >>> fprintf(stderr, "Error-running fill buffer\n"); > >>> ret = -1; > >>> + close(fd_lm); > >>> break; > >>> } > >>> > >> > >> Instead of fixing these existing patterns I think it would make the code > >> easier to understand and maintain if it is made symmetrical. > >> Having the perf event fd opened in one place but its close() > >> scattered elsewhere has the potential for confusion and making later > >> mistakes easy to miss. > >> > >> What if perf event fd is closed in a new "disable_llc_perf()" that > >> is matched with "reset_enable_llc_perf()" and called > >> from cat_val()? > >> > >> I think this raises another issue with the test trickery where > >> measure_cache_vals() has some assumptions about state based on the > >> test name. > > > > I very much agree on the principle here, and thus I already have created > > patches which will do a major cleanup on this area. The cleaned-up code > > has pe_fd local var to cat_val() and handles closing it in cat_val() with > > the usual patterns. > > > > However, the patch is currently resides post L3 CAT test rewrite. > > Backporting the cleanups/refactors into this series would require > > considerable effort due to how convoluted all those n-step cleanup patches > > and L3 CAT test rewrite are in this area. There's just very much to > > cleanup here and L3 rewrite will touch the same areas so its a net > > full of conflicts. > > > > Do you want me to spend the effort to backport them into this series > > (I expect will take some time)? > > Considering the "Fixes" tag, having a smaller fix that can easily > be backported would be ideal so I am ok with deferring a bigger > rework. > > I do think this fix can be made more robust with a couple of small > changes that should not introduce significant conflicts: > * initialize fd_lm to -1 > * do not close() fd_lm in get_llc_perf() but instead move its > close() to at exit of cat_val(). I changed the test to only close the fd in cat_val() which is the direction the later refactor/cleanup changes (not in this series) was moving anyway. > * add check in get_llc_perf() that it does not attempt ioctl() > on "fd_lm == -1" (later addition would be error checking of > the ioctl()) The other two things suggested seem unnecessary and I've not implemented them, I don't thinkg fd_lm can be -1 at ioctl(). Given this code is going to be replaced soonish, putting any extra "safety" effort into it now seems waste of time.
Hi Ilpo, On 7/17/2023 6:05 AM, Ilpo Järvinen wrote: > On Fri, 14 Jul 2023, Reinette Chatre wrote: >> * add check in get_llc_perf() that it does not attempt ioctl() >> on "fd_lm == -1" (later addition would be error checking of >> the ioctl()) > > The other two things suggested seem unnecessary and I've not implemented > them, I don't thinkg fd_lm can be -1 at ioctl(). Given this code is going > to be replaced soonish, putting any extra "safety" effort into it now > seems waste of time. Yes, this suggestion was indeed to make the code more robust. I certainly do not want to waste your time. Please keep in mind when you respond that I do not have insight into the reworks you are still planning. Reinette
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 8a4fe8693be6..ced47b445d1e 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -87,21 +87,20 @@ static int reset_enable_llc_perf(pid_t pid, int cpu_no) static int get_llc_perf(unsigned long *llc_perf_miss) { __u64 total_misses; + int ret; /* Stop counters after one span to get miss rate */ ioctl(fd_lm, PERF_EVENT_IOC_DISABLE, 0); - if (read(fd_lm, &rf_cqm, sizeof(struct read_format)) == -1) { + ret = read(fd_lm, &rf_cqm, sizeof(struct read_format)); + close(fd_lm); + if (ret == -1) { perror("Could not get llc misses through perf"); - return -1; } total_misses = rf_cqm.values[0].value; - - close(fd_lm); - *llc_perf_miss = total_misses; return 0; @@ -253,6 +252,7 @@ int cat_val(struct resctrl_val_param *param) memflush, operation, resctrl_val)) { fprintf(stderr, "Error-running fill buffer\n"); ret = -1; + close(fd_lm); break; }