linux-kernel - Re: [PATCH 2/3] selftests/resctrl: Simplify cleanup in ctrl-c handler

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b7b1aace-66f2-ece8-f8ba-7e323e04fa70@linux.intel.com>
Date: Tue, 20 Feb 2024 16:21:32 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.com>
cc: Fenghua Yu <fenghua.yu@...el.com>, 
    Reinette Chatre <reinette.chatre@...el.com>, Shuah Khan <shuah@...nel.org>, 
    LKML <linux-kernel@...r.kernel.org>, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 2/3] selftests/resctrl: Simplify cleanup in ctrl-c
 handler

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

> On 2024-02-20 at 15:45:23 +0200, Ilpo Järvinen 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.
> >> 
> >> Add a global variable to make ctrl-c handler aware of the currently run
> >> test and only execute the correct cleanup callback.
> >> 
> >> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@...el.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(-)
> >> 
> >> 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;
> >>  
> >>  extern char llc_occup_path[1024];
> >>  
> >> +extern struct resctrl_test current_test;
> >
> >Why this is not just a pointer?
> 
> I tried making this as a pointer but the 'test' in test_prepare() is of type
> 'const struct resctrl_test *' and there are warnings about dropping the const
> modifier.

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

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

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

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

-- 
 i.