| 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
| ||
|
Message-ID: <73344dcef12bbd3a4f6105af43531207f0f24dee.camel@linux.intel.com>
Date: Tue, 02 Jul 2024 23:49:42 -0700
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>, "Rafael J . Wysocki"
<rafael.j.wysocki@...el.com>, Shuah Khan <shuah@...nel.org>
Cc: kernel@...labora.com, linux-kselftest@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] selftests/thermel/intel: conform the test to TAP
output
On Wed, 2024-07-03 at 11:19 +0500, Muhammad Usama Anjum wrote:
> On 7/3/24 9:40 AM, srinivas pandruvada wrote:
> > On Tue, 2024-07-02 at 15:12 +0500, Muhammad Usama Anjum wrote:
> > > Conform the layout, informational and status messages to TAP. No
> > > functional change is intended other than the layout of output
> > > messages.
> > >
> > Not true. You did functional change by adding a break in the loop.
> > The purpose here to wait for these message continuously till ctrl-c
> > or
> > similar.
> I'll correct the description.
>
> >
> > > The test has infinite loop to read the value of status_str. Break
> > > the
> > > loop after getting the value once and finish the test.
> > >
> > No, that is not the purpose.
> We want to finish the test after some iterations as in CIs we cannot
> abort
> the test by sending signals. Would 10 or 100 iterations be enough for
> this
> test? Or it is the kind of test which wouldn't get events until some
> other
> process is using the same driver files?
>
You generate events, you need to do workload and reduce power limit to
trigger. Otherwise these are rare events.
Thanks,
Srinivas
> >
> > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> > > ---
> > > Changes since v1:
> > > - Use ksft_exit_fail_perror if read() returns error
> > > - Break the infinite loop after printing status_str
> > > ---
> > > .../intel/power_floor/power_floor_test.c | 70 ++++++++-----
> > > ----
> > > --
> > > 1 file changed, 30 insertions(+), 40 deletions(-)
> > >
> > > diff --git
> > > a/tools/testing/selftests/thermal/intel/power_floor/power_floor_t
> > > est.
> > > c
> > > b/tools/testing/selftests/thermal/intel/power_floor/power_floor_t
> > > est.
> > > c
> > > index 0326b39a11b91..c06b275acd36b 100644
> > > ---
> > > a/tools/testing/selftests/thermal/intel/power_floor/power_floor_t
> > > est.
> > > c
> > > +++
> > > b/tools/testing/selftests/thermal/intel/power_floor/power_floor_t
> > > est.
> > > c
> > > @@ -9,6 +9,7 @@
> > > #include <fcntl.h>
> > > #include <poll.h>
> > > #include <signal.h>
> > > +#include "../../../kselftest.h"
> > >
> > > #define POWER_FLOOR_ENABLE_ATTRIBUTE
> > > "/sys/bus/pci/devices/0000:00:04.0/power_limits/power_floor_enabl
> > > e"
> > > #define POWER_FLOOR_STATUS_ATTRIBUTE
> > > "/sys/bus/pci/devices/0000:00:04.0/power_limits/power_floor_statu
> > > s"
> > > @@ -20,17 +21,13 @@ void power_floor_exit(int signum)
> > > /* Disable feature via sysfs knob */
> > >
> > > fd = open(POWER_FLOOR_ENABLE_ATTRIBUTE, O_RDWR);
> > > - if (fd < 0) {
> > > - perror("Unable to open power floor enable
> > > file\n");
> > > - exit(1);
> > > - }
> > > + if (fd < 0)
> > > + ksft_exit_fail_perror("Unable to open power
> > > floor
> > > enable file");
> > >
> > > - if (write(fd, "0\n", 2) < 0) {
> > > - perror("Can' disable power floor
> > > notifications\n");
> > > - exit(1);
> > > - }
> > > + if (write(fd, "0\n", 2) < 0)
> > > + ksft_exit_fail_perror("Can' disable power floor
> > > notifications");
> > >
> > > - printf("Disabled power floor notifications\n");
> > > + ksft_print_msg("Disabled power floor notifications\n");
> > >
> > > close(fd);
> > > }
> > > @@ -41,6 +38,9 @@ int main(int argc, char **argv)
> > > char status_str[3];
> > > int fd, ret;
> > >
> > > + ksft_print_header();
> > > + ksft_set_plan(1);
> > > +
> > > if (signal(SIGINT, power_floor_exit) == SIG_IGN)
> > > signal(SIGINT, SIG_IGN);
> > > if (signal(SIGHUP, power_floor_exit) == SIG_IGN)
> > > @@ -50,59 +50,49 @@ int main(int argc, char **argv)
> > >
> > > /* Enable feature via sysfs knob */
> > > fd = open(POWER_FLOOR_ENABLE_ATTRIBUTE, O_RDWR);
> > > - if (fd < 0) {
> > > - perror("Unable to open power floor enable
> > > file\n");
> > > - exit(1);
> > > - }
> > > + if (fd < 0)
> > > + ksft_exit_fail_perror("Unable to open power
> > > floor
> > > enable file");
> > >
> > > - if (write(fd, "1\n", 2) < 0) {
> > > - perror("Can' enable power floor
> > > notifications\n");
> > > - exit(1);
> > > - }
> > > + if (write(fd, "1\n", 2) < 0)
> > > + ksft_exit_fail_perror("Can' enable power floor
> > > notifications");
> > >
> > > close(fd);
> > >
> > > - printf("Enabled power floor notifications\n");
> > > + ksft_print_msg("Enabled power floor notifications\n");
> > >
> > > while (1) {
> > > fd = open(POWER_FLOOR_STATUS_ATTRIBUTE,
> > > O_RDONLY);
> > > - if (fd < 0) {
> > > - perror("Unable to power floor status
> > > file\n");
> > > - exit(1);
> > > - }
> > > + if (fd < 0)
> > > + ksft_exit_fail_perror("Unable to power
> > > floor
> > > status file");
> > >
> > > - if ((lseek(fd, 0L, SEEK_SET)) < 0) {
> > > - fprintf(stderr, "Failed to set pointer
> > > to
> > > beginning\n");
> > > - exit(1);
> > > - }
> > > + if ((lseek(fd, 0L, SEEK_SET)) < 0)
> > > + ksft_exit_fail_perror("Failed to set
> > > pointer
> > > to beginning\n");
> > >
> > > - if (read(fd, status_str, sizeof(status_str)) <
> > > 0) {
> > > - fprintf(stderr, "Failed to read
> > > from:%s\n",
> > > - POWER_FLOOR_STATUS_ATTRIBUTE);
> > > - exit(1);
> > > - }
> > > + if (read(fd, status_str, sizeof(status_str)) <
> > > 0)
> > > + ksft_exit_fail_perror("Failed to read
> > > from:
> > > power_floor_status");
> > >
> > > ufd.fd = fd;
> > > ufd.events = POLLPRI;
> > >
> > > ret = poll(&ufd, 1, -1);
> > > if (ret < 0) {
> > > - perror("poll error");
> > > - exit(1);
> > > + ksft_exit_fail_msg("Poll error\n");
> > > } else if (ret == 0) {
> > > - printf("Poll Timeout\n");
> > > + ksft_print_msg("Poll Timeout\n");
> > > } else {
> > > - if ((lseek(fd, 0L, SEEK_SET)) < 0) {
> > > - fprintf(stderr, "Failed to set
> > > pointer to beginning\n");
> > > - exit(1);
> > > - }
> > > + if ((lseek(fd, 0L, SEEK_SET)) < 0)
> > > + ksft_exit_fail_msg("Failed to
> > > set
> > > pointer to beginning\n");
> > >
> > > if (read(fd, status_str,
> > > sizeof(status_str))
> > > < 0)
> > > - exit(0);
> > > + ksft_exit_fail_perror("Failed to
> > > read");
> > >
> > > - printf("power floor status: %s\n",
> > > status_str);
> > > + ksft_print_msg("power floor status:
> > > %s\n",
> > > status_str);
> > > + break;
> > > }
> > >
> > > close(fd);
> > > }
> > > +
> > > + ksft_test_result_pass("Successfully read\n");
> > > + ksft_finished();
> > > }
> >
> >
>
Powered by blists - more mailing lists