* [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 6:00 ` Li Wang
2021-09-06 7:54 ` xuyang2018.jy
2021-09-06 8:59 ` Cyril Hrubis
0 siblings, 2 replies; 6+ messages in thread
From: Li Wang @ 2021-09-06 6:00 UTC (permalink / raw)
To: ltp
The st_mtime field is defined as st_mtim.tv_sec for backward
compatibility in struct stat, which might not precise enough
for timestamp comparing.
Here switch to timespec diff (with compare nanosecond as well) to
get rid of this kind of rare faliure:
7 tst_test.c:1345: TINFO: Timeout per run is 0h 05m 00s
8 copy_file_range.h:36: TINFO: Testing libc copy_file_range()
9 copy_file_range03.c:48: TPASS: copy_file_range sucessfully updated the timestamp
10 tst_test.c:1345: TINFO: Timeout per run is 0h 05m 00s
11 copy_file_range.h:39: TINFO: Testing __NR_copy_file_range syscall
12 copy_file_range03.c:46: TFAIL: copy_file_range did not update timestamp.
Also, raise the sleep time to 1.5 sec to make test more robust.
Signed-off-by: Li Wang <liwang@redhat.com>
---
.../copy_file_range/copy_file_range03.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
index 253eb57ad..5950c80c1 100644
--- a/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
+++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
@@ -12,26 +12,27 @@
#define _GNU_SOURCE
#include "tst_test.h"
+#include "tst_timer.h"
#include "copy_file_range.h"
static int fd_src;
static int fd_dest;
-unsigned long get_timestamp(int fd)
+struct timespec get_timestamp(int fd)
{
struct stat filestat;
fstat(fd, &filestat);
- return filestat.st_mtime;
+ return filestat.st_mtim;
}
static void verify_copy_file_range_timestamp(void)
{
loff_t offset;
- unsigned long timestamp, updated_timestamp;
+ struct timespec timestamp1, timestamp2, diff;
- timestamp = get_timestamp(fd_dest);
- usleep(1000000);
+ timestamp1 = get_timestamp(fd_dest);
+ usleep(1500000);
offset = 0;
TEST(sys_copy_file_range(fd_src, &offset,
@@ -40,9 +41,11 @@ static void verify_copy_file_range_timestamp(void)
tst_brk(TBROK | TTERRNO,
"copy_file_range unexpectedly failed");
- updated_timestamp = get_timestamp(fd_dest);
+ timestamp2 = get_timestamp(fd_dest);
- if (timestamp == updated_timestamp)
+ diff = tst_timespec_diff(timestamp1, timestamp2);
+
+ if (!diff.tv_sec && !diff.tv_nsec)
tst_brk(TFAIL, "copy_file_range did not update timestamp.");
tst_res(TPASS, "copy_file_range sucessfully updated the timestamp");
--
2.31.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 7:54 ` xuyang2018.jy
0 siblings, 0 replies; 6+ messages in thread
From: xuyang2018.jy @ 2021-09-06 7:54 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi Li
Looks good to me.
Acked-by: Yang Xu <xuyang2018.jy@fujitsu.com>
Best Regards
Yang Xu
> The st_mtime field is defined as st_mtim.tv_sec for backward
> compatibility in struct stat, which might not precise enough
> for timestamp comparing.
>
> Here switch to timespec diff (with compare nanosecond as well) to
> get rid of this kind of rare faliure:
>
> 7 tst_test.c:1345: TINFO: Timeout per run is 0h 05m 00s
> 8 copy_file_range.h:36: TINFO: Testing libc copy_file_range()
> 9 copy_file_range03.c:48: TPASS: copy_file_range sucessfully updated the timestamp
> 10 tst_test.c:1345: TINFO: Timeout per run is 0h 05m 00s
> 11 copy_file_range.h:39: TINFO: Testing __NR_copy_file_range syscall
> 12 copy_file_range03.c:46: TFAIL: copy_file_range did not update timestamp.
>
> Also, raise the sleep time to 1.5 sec to make test more robust.
>
> Signed-off-by: Li Wang<liwang@redhat.com>
> ---
> .../copy_file_range/copy_file_range03.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
> index 253eb57ad..5950c80c1 100644
> --- a/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
> +++ b/testcases/kernel/syscalls/copy_file_range/copy_file_range03.c
> @@ -12,26 +12,27 @@
> #define _GNU_SOURCE
>
> #include "tst_test.h"
> +#include "tst_timer.h"
> #include "copy_file_range.h"
>
> static int fd_src;
> static int fd_dest;
>
> -unsigned long get_timestamp(int fd)
> +struct timespec get_timestamp(int fd)
> {
> struct stat filestat;
>
> fstat(fd,&filestat);
> - return filestat.st_mtime;
> + return filestat.st_mtim;
> }
>
> static void verify_copy_file_range_timestamp(void)
> {
> loff_t offset;
> - unsigned long timestamp, updated_timestamp;
> + struct timespec timestamp1, timestamp2, diff;
>
> - timestamp = get_timestamp(fd_dest);
> - usleep(1000000);
> + timestamp1 = get_timestamp(fd_dest);
> + usleep(1500000);
>
> offset = 0;
> TEST(sys_copy_file_range(fd_src,&offset,
> @@ -40,9 +41,11 @@ static void verify_copy_file_range_timestamp(void)
> tst_brk(TBROK | TTERRNO,
> "copy_file_range unexpectedly failed");
>
> - updated_timestamp = get_timestamp(fd_dest);
> + timestamp2 = get_timestamp(fd_dest);
>
> - if (timestamp == updated_timestamp)
> + diff = tst_timespec_diff(timestamp1, timestamp2);
> +
> + if (!diff.tv_sec&& !diff.tv_nsec)
> tst_brk(TFAIL, "copy_file_range did not update timestamp.");
>
> tst_res(TPASS, "copy_file_range sucessfully updated the timestamp");
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 8:59 ` Cyril Hrubis
2021-09-06 10:35 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2021-09-06 8:59 UTC (permalink / raw)
To: Li Wang; +Cc: ltp
Hi!
> - updated_timestamp = get_timestamp(fd_dest);
> + timestamp2 = get_timestamp(fd_dest);
>
> - if (timestamp == updated_timestamp)
> + diff = tst_timespec_diff(timestamp1, timestamp2);
> +
> + if (!diff.tv_sec && !diff.tv_nsec)
> tst_brk(TFAIL, "copy_file_range did not update timestamp.");
So as we changed the code to sleep for 1.5 sec I guess that we can
expect the difference to be at least 1 second because:
- the minimal granularity is 1s in which case we will get exactly 1s in
the diff
- if the granularity is greater, we will get a bit more than 1s
So I would go for something as:
long long diff_us = tst_timespec_diff_us(timestamp2, timestamp1);
if (diff_us >= 1000000 && diff_us <= 2000000)
tst_res(TPASS, "...");
Which expects that the difference between timestamps is in a sane range
not that it just have been changed.
> tst_res(TPASS, "copy_file_range sucessfully updated the timestamp");
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 10:35 ` Li Wang
2021-09-06 10:38 ` Cyril Hrubis
0 siblings, 1 reply; 6+ messages in thread
From: Li Wang @ 2021-09-06 10:35 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: LTP List
[-- Attachment #1.1: Type: text/plain, Size: 1562 bytes --]
On Mon, Sep 6, 2021 at 4:59 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > - updated_timestamp = get_timestamp(fd_dest);
> > + timestamp2 = get_timestamp(fd_dest);
> >
> > - if (timestamp == updated_timestamp)
> > + diff = tst_timespec_diff(timestamp1, timestamp2);
> > +
> > + if (!diff.tv_sec && !diff.tv_nsec)
> > tst_brk(TFAIL, "copy_file_range did not update
> timestamp.");
>
> So as we changed the code to sleep for 1.5 sec I guess that we can
> expect the difference to be at least 1 second because:
>
> - the minimal granularity is 1s in which case we will get exactly 1s in
> the diff
>
> - if the granularity is greater, we will get a bit more than 1s
>
> So I would go for something as:
>
> long long diff_us = tst_timespec_diff_us(timestamp2, timestamp1);
>
> if (diff_us >= 1000000 && diff_us <= 2000000)
> tst_res(TPASS, "...");
>
> Which expects that the difference between timestamps is in a sane range
> not that it just have been changed.
>
Theoretically, this is correct, but I'm fearing that the process might
cost more time on performing with different system loads.
This means 'diff_us <= 2000000' is an unreliable condition to
many virtual machines. That largely depends on the system
scheduler at that moment as well.
>
> > tst_res(TPASS, "copy_file_range sucessfully updated the
> timestamp");
> > --
> > 2.31.1
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
>
--
Regards,
Li Wang
[-- Attachment #1.2: Type: text/html, Size: 2983 bytes --]
[-- Attachment #2: Type: text/plain, Size: 60 bytes --]
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 10:38 ` Cyril Hrubis
2021-09-06 10:42 ` Li Wang
0 siblings, 1 reply; 6+ messages in thread
From: Cyril Hrubis @ 2021-09-06 10:38 UTC (permalink / raw)
To: Li Wang; +Cc: LTP List
Hi!
> Theoretically, this is correct, but I'm fearing that the process might
> cost more time on performing with different system loads.
>
> This means 'diff_us <= 2000000' is an unreliable condition to
> many virtual machines. That largely depends on the system
> scheduler at that moment as well.
Right, I guess that we can relax the upper bound, it can be even a
minute and it would still be a good sanity check.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff
@ 2021-09-06 10:42 ` Li Wang
0 siblings, 0 replies; 6+ messages in thread
From: Li Wang @ 2021-09-06 10:42 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: LTP List
[-- Attachment #1.1: Type: text/plain, Size: 626 bytes --]
On Mon, Sep 6, 2021 at 6:37 PM Cyril Hrubis <chrubis@suse.cz> wrote:
> Hi!
> > Theoretically, this is correct, but I'm fearing that the process might
> > cost more time on performing with different system loads.
> >
> > This means 'diff_us <= 2000000' is an unreliable condition to
> > many virtual machines. That largely depends on the system
> > scheduler at that moment as well.
>
> Right, I guess that we can relax the upper bound, it can be even a
> minute and it would still be a good sanity check.
>
Yes, I'd relax it to 30 sec to see how it performs in real scenarios.
Patch v3 is on the way.
--
Regards,
Li Wang
[-- Attachment #1.2: Type: text/html, Size: 1301 bytes --]
[-- Attachment #2: Type: text/plain, Size: 60 bytes --]
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-06 10:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-06 6:00 [LTP] [PATCH v2] copy_file_range03: comparing timestamp in tst_timespec_diff Li Wang
2021-09-06 6:00 ` Li Wang
2021-09-06 7:54 ` xuyang2018.jy
2021-09-06 7:54 ` xuyang2018.jy
2021-09-06 8:59 ` Cyril Hrubis
2021-09-06 8:59 ` Cyril Hrubis
2021-09-06 10:35 ` Li Wang
2021-09-06 10:35 ` Li Wang
2021-09-06 10:38 ` Cyril Hrubis
2021-09-06 10:38 ` Cyril Hrubis
2021-09-06 10:42 ` Li Wang
2021-09-06 10:42 ` Li Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.