Received-SPF: pass (google.com: domain of linux-kernel+bounces-73174-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 20 Feb 2024 16:21:32 +0200 (EET)
To: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
cc: Fenghua Yu <fenghua.yu@intel.com>, 
    Reinette Chatre <reinette.chatre@intel.com>, Shuah Khan <shuah@kernel.org>, 
    LKML <linux-kernel@vger.kernel.org>, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH 2/3] selftests/resctrl: Simplify cleanup in ctrl-c
 handler
In-Reply-To: <uck6k4upde5642uarjjyoeyt76ju7f3ympqbnugnv3ufaw65k2@wdh2chxqsooo>
Message-ID: <b7b1aace-66f2-ece8-f8ba-7e323e04fa70@linux.intel.com>
References: <cover.1708434017.git.maciej.wieczor-retman@intel.com> <8c4fcfb6b4e38a0f0e400be88ecf1af0d20e12e7.1708434017.git.maciej.wieczor-retman@intel.com> <5f251bcd-a343-bb6e-a947-7605dc59f9ea@linux.intel.com>
 <uck6k4upde5642uarjjyoeyt76ju7f3ympqbnugnv3ufaw65k2@wdh2chxqsooo>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-1414670152-1708438892=:1099"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-1414670152-1708438892=:1099
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Tue, 20 Feb 2024, Maciej Wieczor-Retman wrote:

> On 2024-02-20 at 15:45:23 +0200, Ilpo J=E4rvinen wrote:
> >On Tue, 20 Feb 2024, Maciej Wieczor-Retman wrote:
> >
> >> Ctrl-c handler isn't aware of what test is currently running. Because =
of
> >> that it executes all cleanups even if they aren't necessary. Since the
> >> ctrl-c handler uses the sigaction system no parameters can be passed
> >> to it as function arguments.
> >>=20
> >> Add a global variable to make ctrl-c handler aware of the currently ru=
n
> >> test and only execute the correct cleanup callback.
> >>=20
> >> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
> >> ---
> >>  tools/testing/selftests/resctrl/resctrl.h     |  2 ++
> >>  .../testing/selftests/resctrl/resctrl_tests.c | 20 +++++++++---------=
-
> >>  tools/testing/selftests/resctrl/resctrl_val.c |  2 +-
> >>  3 files changed, 13 insertions(+), 11 deletions(-)
> >>=20
> >> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing=
/selftests/resctrl/resctrl.h
> >> index 0f49df4961ea..79b45cbeb628 100644
> >> --- a/tools/testing/selftests/resctrl/resctrl.h
> >> +++ b/tools/testing/selftests/resctrl/resctrl.h
> >> @@ -128,6 +128,8 @@ extern pid_t bm_pid, ppid;
> >> =20
> >>  extern char llc_occup_path[1024];
> >> =20
> >> +extern struct resctrl_test current_test;
> >
> >Why this is not just a pointer?
>=20
> I tried making this as a pointer but the 'test' in test_prepare() is of t=
ype
> 'const struct resctrl_test *' and there are warnings about dropping the c=
onst
> modifier.

Why cannot the pointer be const too? The signal handler is not supposed to=
=20
modify the contents of the resctrl_test struct.

There are two types of constness in C. One (the most commonly used one)=20
relates to immutability of the contents of the struct the pointer (or char=
=20
*) points to while the other enforces the pointer itself to remain=20
immutable. Usually, the former is what is useful and it's what you get=20
when you naturally write "const struct".

> >> +=09current_test =3D *test;
> >
> >I'd prefer to keep this internal to signal handling functions so that=20
> >either the struct resctrl_test or just the cleanup handler is passed=20
> >to signal_handler_register().
>=20
> Okay, would moving this assignment to signal_handler_register() be okay t=
hen?

Yes, that's what I'm after here. Lets keep it internal to the signal=20
handling code.

--=20
 i.

--8323328-1414670152-1708438892=:1099--