| Message ID | 20240702101259.1251377-1-usama.anjum@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] selftests/thermel/intel: conform the test to TAP output | expand |
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. > > The test has infitie infinite > loop to read the value of index_str. Break the loop > after successfully reading the value once and finished the test. > That is not correct. This loops till SIGINT | SIGTERM | SIGHUP Anyway this comment is not relevant for what you are doing here. > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> Thanks, Srinivas > --- > Changes since v1: > - Use ksft_exit_fail_perror if read() returns error > - Break the infinite loop after printing index_str > --- > .../intel/workload_hint/workload_hint_test.c | 103 ++++++++-------- > -- > 1 file changed, 43 insertions(+), 60 deletions(-) > > diff --git > a/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_t > est.c > b/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_t > est.c > index 217c3a641c537..0e5f07efc8a2b 100644 > --- > a/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_t > est.c > +++ > b/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_t > est.c > @@ -9,6 +9,7 @@ > #include <fcntl.h> > #include <poll.h> > #include <signal.h> > +#include "../../../kselftest.h" > > #define WORKLOAD_NOTIFICATION_DELAY_ATTRIBUTE > "/sys/bus/pci/devices/0000:00:04.0/workload_hint/notification_delay_m > s" > #define WORKLOAD_ENABLE_ATTRIBUTE > "/sys/bus/pci/devices/0000:00:04.0/workload_hint/workload_hint_enable > " > @@ -31,17 +32,13 @@ void workload_hint_exit(int signum) > /* Disable feature via sysfs knob */ > > fd = open(WORKLOAD_ENABLE_ATTRIBUTE, O_RDWR); > - if (fd < 0) { > - perror("Unable to open workload type feature enable > file\n"); > - exit(1); > - } > + if (fd < 0) > + ksft_exit_fail_perror("Unable to open workload type > feature enable file"); > > - if (write(fd, "0\n", 2) < 0) { > - perror("Can' disable workload hints\n"); > - exit(1); > - } > + if (write(fd, "0\n", 2) < 0) > + ksft_exit_fail_perror("Can' disable workload > hints"); > > - printf("Disabled workload type prediction\n"); > + ksft_print_msg("Disabled workload type prediction\n"); > > close(fd); > } > @@ -54,32 +51,27 @@ int main(int argc, char **argv) > char delay_str[64]; > int delay = 0; > > - printf("Usage: workload_hint_test [notification delay in > milli seconds]\n"); > + ksft_print_header(); > + ksft_set_plan(1); > + > + ksft_print_msg("Usage: workload_hint_test [notification > delay in milli seconds]\n"); > > if (argc > 1) { > ret = sscanf(argv[1], "%d", &delay); > - if (ret < 0) { > - printf("Invalid delay\n"); > - exit(1); > - } > + if (ret < 0) > + ksft_exit_fail_perror("Invalid delay"); > > - printf("Setting notification delay to %d ms\n", > delay); > + ksft_print_msg("Setting notification delay to %d > ms\n", delay); > if (delay < 0) > - exit(1); > - > - sprintf(delay_str, "%s\n", argv[1]); > + ksft_exit_fail_msg("delay can never be > negative\n"); > > sprintf(delay_str, "%s\n", argv[1]); > fd = open(WORKLOAD_NOTIFICATION_DELAY_ATTRIBUTE, > O_RDWR); > - if (fd < 0) { > - perror("Unable to open workload notification > delay\n"); > - exit(1); > - } > + if (fd < 0) > + ksft_exit_fail_perror("Unable to open > workload notification delay"); > > - if (write(fd, delay_str, strlen(delay_str)) < 0) { > - perror("Can't set delay\n"); > - exit(1); > - } > + if (write(fd, delay_str, strlen(delay_str)) < 0) > + ksft_exit_fail_perror("Can't set delay"); > > close(fd); > } > @@ -93,65 +85,56 @@ int main(int argc, char **argv) > > /* Enable feature via sysfs knob */ > fd = open(WORKLOAD_ENABLE_ATTRIBUTE, O_RDWR); > - if (fd < 0) { > - perror("Unable to open workload type feature enable > file\n"); > - exit(1); > - } > + if (fd < 0) > + ksft_exit_fail_perror("Unable to open workload type > feature enable file"); > > - if (write(fd, "1\n", 2) < 0) { > - perror("Can' enable workload hints\n"); > - exit(1); > - } > + if (write(fd, "1\n", 2) < 0) > + ksft_exit_fail_perror("Can' enable workload hints"); > > close(fd); > > - printf("Enabled workload type prediction\n"); > + ksft_print_msg("Enabled workload type prediction\n"); > > while (1) { > fd = open(WORKLOAD_TYPE_INDEX_ATTRIBUTE, O_RDONLY); > - if (fd < 0) { > - perror("Unable to open workload type > file\n"); > - exit(1); > - } > + if (fd < 0) > + ksft_exit_fail_perror("Unable to open > workload type 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"); > > - if (read(fd, index_str, sizeof(index_str)) < 0) { > - fprintf(stderr, "Failed to read from:%s\n", > - WORKLOAD_TYPE_INDEX_ATTRIBUTE); > - exit(1); > - } > + if (read(fd, index_str, sizeof(index_str)) < 0) > + ksft_exit_fail_perror("Failed to read from: > workload_type_index"); > > ufd.fd = fd; > ufd.events = POLLPRI; > > ret = poll(&ufd, 1, -1); > if (ret < 0) { > - perror("poll error"); > - exit(1); > + ksft_exit_fail_perror("poll error"); > } 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_perror("Failed to set > pointer to beginning"); > > if (read(fd, index_str, sizeof(index_str)) < > 0) > - exit(0); > + ksft_exit_fail_perror("Failed to > read"); > > ret = sscanf(index_str, "%d", &index); > if (ret < 0) > + ksft_exit_fail_msg("Read negative > value unexpectedly\n"); > + if (index > WORKLOAD_TYPE_MAX_INDEX) { > + ksft_print_msg("Invalid workload > type index\n"); > + } else { > + ksft_print_msg("workload type:%s\n", > workload_types[index]); > break; > - if (index > WORKLOAD_TYPE_MAX_INDEX) > - printf("Invalid workload type > index\n"); > - else > - printf("workload type:%s\n", > workload_types[index]); > + } > } > > close(fd); > } > + > + ksft_test_result_pass("Successfully read\n"); > + ksft_finished(); > }
On 7/2/24 04:12, 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. > > The test has infitie loop to read the value of index_str. Break the loop > after successfully reading the value once and finished the test. > > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > Changes since v1: > - Use ksft_exit_fail_perror if read() returns error > - Break the infinite loop after printing index_str > --- Usama, As I mentioned another TAP conversion patch from you patch if the following command gives you TAP, there is no need to convert. make -C tools/testing/tmp2 run_tests make kselftest TARGETS=tmp2 kselftest framework lib.mk and runtests wrappers take care for TAP. The reason to take care of this at framework level is to avoid changes to individual tests. The wrapper keys off of KSFT_* codes returned from tests. Please don't send TAP conversion patches like this one. The output from the commands will have duplicate messages. The reason tests return make -C tools/testing/tmp2 run_tests make kselftest TARGETS=tmp2 thanks, -- Shuah
diff --git a/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c b/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c index 217c3a641c537..0e5f07efc8a2b 100644 --- a/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c +++ b/tools/testing/selftests/thermal/intel/workload_hint/workload_hint_test.c @@ -9,6 +9,7 @@ #include <fcntl.h> #include <poll.h> #include <signal.h> +#include "../../../kselftest.h" #define WORKLOAD_NOTIFICATION_DELAY_ATTRIBUTE "/sys/bus/pci/devices/0000:00:04.0/workload_hint/notification_delay_ms" #define WORKLOAD_ENABLE_ATTRIBUTE "/sys/bus/pci/devices/0000:00:04.0/workload_hint/workload_hint_enable" @@ -31,17 +32,13 @@ void workload_hint_exit(int signum) /* Disable feature via sysfs knob */ fd = open(WORKLOAD_ENABLE_ATTRIBUTE, O_RDWR); - if (fd < 0) { - perror("Unable to open workload type feature enable file\n"); - exit(1); - } + if (fd < 0) + ksft_exit_fail_perror("Unable to open workload type feature enable file"); - if (write(fd, "0\n", 2) < 0) { - perror("Can' disable workload hints\n"); - exit(1); - } + if (write(fd, "0\n", 2) < 0) + ksft_exit_fail_perror("Can' disable workload hints"); - printf("Disabled workload type prediction\n"); + ksft_print_msg("Disabled workload type prediction\n"); close(fd); } @@ -54,32 +51,27 @@ int main(int argc, char **argv) char delay_str[64]; int delay = 0; - printf("Usage: workload_hint_test [notification delay in milli seconds]\n"); + ksft_print_header(); + ksft_set_plan(1); + + ksft_print_msg("Usage: workload_hint_test [notification delay in milli seconds]\n"); if (argc > 1) { ret = sscanf(argv[1], "%d", &delay); - if (ret < 0) { - printf("Invalid delay\n"); - exit(1); - } + if (ret < 0) + ksft_exit_fail_perror("Invalid delay"); - printf("Setting notification delay to %d ms\n", delay); + ksft_print_msg("Setting notification delay to %d ms\n", delay); if (delay < 0) - exit(1); - - sprintf(delay_str, "%s\n", argv[1]); + ksft_exit_fail_msg("delay can never be negative\n"); sprintf(delay_str, "%s\n", argv[1]); fd = open(WORKLOAD_NOTIFICATION_DELAY_ATTRIBUTE, O_RDWR); - if (fd < 0) { - perror("Unable to open workload notification delay\n"); - exit(1); - } + if (fd < 0) + ksft_exit_fail_perror("Unable to open workload notification delay"); - if (write(fd, delay_str, strlen(delay_str)) < 0) { - perror("Can't set delay\n"); - exit(1); - } + if (write(fd, delay_str, strlen(delay_str)) < 0) + ksft_exit_fail_perror("Can't set delay"); close(fd); } @@ -93,65 +85,56 @@ int main(int argc, char **argv) /* Enable feature via sysfs knob */ fd = open(WORKLOAD_ENABLE_ATTRIBUTE, O_RDWR); - if (fd < 0) { - perror("Unable to open workload type feature enable file\n"); - exit(1); - } + if (fd < 0) + ksft_exit_fail_perror("Unable to open workload type feature enable file"); - if (write(fd, "1\n", 2) < 0) { - perror("Can' enable workload hints\n"); - exit(1); - } + if (write(fd, "1\n", 2) < 0) + ksft_exit_fail_perror("Can' enable workload hints"); close(fd); - printf("Enabled workload type prediction\n"); + ksft_print_msg("Enabled workload type prediction\n"); while (1) { fd = open(WORKLOAD_TYPE_INDEX_ATTRIBUTE, O_RDONLY); - if (fd < 0) { - perror("Unable to open workload type file\n"); - exit(1); - } + if (fd < 0) + ksft_exit_fail_perror("Unable to open workload type 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"); - if (read(fd, index_str, sizeof(index_str)) < 0) { - fprintf(stderr, "Failed to read from:%s\n", - WORKLOAD_TYPE_INDEX_ATTRIBUTE); - exit(1); - } + if (read(fd, index_str, sizeof(index_str)) < 0) + ksft_exit_fail_perror("Failed to read from: workload_type_index"); ufd.fd = fd; ufd.events = POLLPRI; ret = poll(&ufd, 1, -1); if (ret < 0) { - perror("poll error"); - exit(1); + ksft_exit_fail_perror("poll error"); } 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_perror("Failed to set pointer to beginning"); if (read(fd, index_str, sizeof(index_str)) < 0) - exit(0); + ksft_exit_fail_perror("Failed to read"); ret = sscanf(index_str, "%d", &index); if (ret < 0) + ksft_exit_fail_msg("Read negative value unexpectedly\n"); + if (index > WORKLOAD_TYPE_MAX_INDEX) { + ksft_print_msg("Invalid workload type index\n"); + } else { + ksft_print_msg("workload type:%s\n", workload_types[index]); break; - if (index > WORKLOAD_TYPE_MAX_INDEX) - printf("Invalid workload type index\n"); - else - printf("workload type:%s\n", workload_types[index]); + } } close(fd); } + + ksft_test_result_pass("Successfully read\n"); + ksft_finished(); }
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. The test has infitie loop to read the value of index_str. Break the loop after successfully reading the value once and finished the test. Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com> --- Changes since v1: - Use ksft_exit_fail_perror if read() returns error - Break the infinite loop after printing index_str --- .../intel/workload_hint/workload_hint_test.c | 103 ++++++++---------- 1 file changed, 43 insertions(+), 60 deletions(-)