diff mbox series

[06/10] copy_file_range01: Add max_runtime

Message ID 20220830135007.16818-7-mdoucha@suse.cz
State Changes Requested
Headers show
Series Max_runtime and other minor fixes | expand

Commit Message

Martin Doucha Aug. 30, 2022, 1:50 p.m. UTC
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/syscalls/copy_file_range/copy_file_range01.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Petr Vorel Aug. 31, 2022, 8:21 a.m. UTC | #1
Hi Martin,

LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
Cyril Hrubis Aug. 31, 2022, 9:06 a.m. UTC | #2
Hi!
> LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).

The runtime applies per single call of the run() function.
Martin Doucha Aug. 31, 2022, 9:06 a.m. UTC | #3
On 31. 08. 22 10:21, Petr Vorel wrote:
> Hi Martin,
> 
> LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).
Hmm, I've missed the .all_filesystems setting here. In that case the 
test should take ~2 seconds per filesystems and the patch is not needed. 
Feel free to discard it then.
Cyril Hrubis Aug. 31, 2022, 9:22 a.m. UTC | #4
Hi!
> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  testcases/kernel/syscalls/copy_file_range/copy_file_range01.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
> index 7d27007a3..23a5ec501 100644
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
> @@ -232,4 +232,5 @@ static struct tst_test test = {
>  	.all_filesystems = 1,
>  	.test = copy_file_range_verify,
>  	.test_variants = TEST_VARIANTS,
> +	.max_runtime = 20
>  };

I would still make the loops that copy and read the data runtime aware
to make the error messages more obvious. In the case that you want to
keep TBROK return value it would be:

diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
index 7d27007a3..9b39554d6 100644
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
@@ -52,6 +52,10 @@ static int check_file_content(const char *fname1, const char *fname2,
                ch1 = getc(fp1);
                ch2 = getc(fp2);
                count++;
+
+               if (!(count%100) && !tst_remaining_runtime())
+                       tst_brk(TBROK, "Out of runtime, exiting");
+
        } while ((count < len) && (ch1 == ch2));

        SAFE_FCLOSE(fp1);
@@ -135,6 +139,9 @@ static void test_one(size_t len, loff_t *off_in, loff_t *off_out, char *path)
                        return;
                }

+               if (!tst_remaining_runtime())
+                       tst_brk(TBROK, "Out of runtime, exiting");
+
                to_copy -= TST_RET;
        } while (to_copy > 0);

@@ -230,6 +237,7 @@ static struct tst_test test = {
        .mount_device = 1,
        .mntpoint = MNTPOINT,
        .all_filesystems = 1,
+       .max_runtime = 20,
        .test = copy_file_range_verify,
        .test_variants = TEST_VARIANTS,
 };
Cyril Hrubis Aug. 31, 2022, 9:25 a.m. UTC | #5
Hi!
> > LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).
> Hmm, I've missed the .all_filesystems setting here. In that case the 
> test should take ~2 seconds per filesystems and the patch is not needed. 
> Feel free to discard it then.

I would still vote for adding something as .max_runtime = 5 or something
like this so that it's obvious that the test can run for a few seconds
per run() call especially on slower disks.
Petr Vorel Aug. 31, 2022, 12:18 p.m. UTC | #6
> Hi!
> > LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).

> The runtime applies per single call of the run() function.

Thanks for a hint Cyril. I didn't read the sources correctly before. Now I see
alarm(results->timeout) in fork_testrun() which is called for each filesystem.

Kind regards,
Petr
Petr Vorel Aug. 31, 2022, 12:19 p.m. UTC | #7
> Hi!
> > > LGTM (test uses all_filesystems, but 20 for all possible filesystems should be OK).
> > Hmm, I've missed the .all_filesystems setting here. In that case the 
> > test should take ~2 seconds per filesystems and the patch is not needed. 
> > Feel free to discard it then.

> I would still vote for adding something as .max_runtime = 5 or something
> like this so that it's obvious that the test can run for a few seconds
> per run() call especially on slower disks.

+1

Kind regards,
Petr
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
index 7d27007a3..23a5ec501 100644
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range01.c
@@ -232,4 +232,5 @@  static struct tst_test test = {
 	.all_filesystems = 1,
 	.test = copy_file_range_verify,
 	.test_variants = TEST_VARIANTS,
+	.max_runtime = 20
 };