Message ID | 20230209090307.491586-1-pifang@redhat.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [v1] aiocp: Filter out O_DIRECT before read | expand |
Hi! > When aiocp executed with -f DIRECT will fail. > > <<<test_start>>> > tag=AD049 stime=1675520824 > cmdline="aiocp -b 8k -n 8 -f DIRECT" > contacts="" > analysis=exit > <<<test_output>>> > tst_test.c:1560: TINFO: Timeout per run is 0h 30m 30s > aiocp.c:211: TINFO: Maximum AIO blocks: 65536 > tst_device.c:585: TINFO: Use uevent strategy > aiocp.c:250: TINFO: Fill srcfile.bin with random data > aiocp.c:279: TINFO: Copy srcfile.bin -> dstfile.bin > aiocp.c:291: TINFO: Comparing srcfile.bin with dstfile.bin > aiocp.c:306: TBROK: read(3,0x7ffcd743abe0,4096) failed, returned -1: EINVAL (22) > ... > > syscall read manual ERROR section said that: > EINVAL fd is attached to an object which is unsuitable for reading; > or the file was opened with the O_DIRECT flag, and either the address > specified in buf, the value specified in count, or the file offset is > not suitably aligned. > > We need filter out O_DIRECT flag before read. This is not very good changelog, I had to look closely at the source to figure out why we may need this. Better description should say that the code which checks that the data has been written correctly does not use aligned buffers, which may cause a failure like the one above. > Signed-off-by: Ping Fang <pifang@redhat.com> > --- > testcases/kernel/io/ltp-aiodio/aiocp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/io/ltp-aiodio/aiocp.c b/testcases/kernel/io/ltp-aiodio/aiocp.c > index bc0e209b2..e4252d641 100644 > --- a/testcases/kernel/io/ltp-aiodio/aiocp.c > +++ b/testcases/kernel/io/ltp-aiodio/aiocp.c > @@ -297,8 +297,8 @@ static void run(void) > return; > } > > - srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666); > - dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666); > + srcfd = SAFE_OPEN(srcname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); > + dstfd = SAFE_OPEN(dstname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); I guess that we can as well just remove the srcflags and keep just the O_RDONLY since the srcflags are by definition either O_RDONLY or O_DIRECT | O_RDONLY. I suppose that using scrflags and dstflags for anything else than the filedescriptors passed to the async_run() is actually a mistake. > reads = howmany(filesize, buffsize); > > -- > 2.31.1 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi, On 2/9/23 12:06, Cyril Hrubis wrote: > Hi! >> When aiocp executed with -f DIRECT will fail. >> >> <<<test_start>>> >> tag=AD049 stime=1675520824 >> cmdline="aiocp -b 8k -n 8 -f DIRECT" >> contacts="" >> analysis=exit >> <<<test_output>>> >> tst_test.c:1560: TINFO: Timeout per run is 0h 30m 30s >> aiocp.c:211: TINFO: Maximum AIO blocks: 65536 >> tst_device.c:585: TINFO: Use uevent strategy >> aiocp.c:250: TINFO: Fill srcfile.bin with random data >> aiocp.c:279: TINFO: Copy srcfile.bin -> dstfile.bin >> aiocp.c:291: TINFO: Comparing srcfile.bin with dstfile.bin >> aiocp.c:306: TBROK: read(3,0x7ffcd743abe0,4096) failed, returned -1: EINVAL (22) >> ... >> >> syscall read manual ERROR section said that: >> EINVAL fd is attached to an object which is unsuitable for reading; >> or the file was opened with the O_DIRECT flag, and either the address >> specified in buf, the value specified in count, or the file offset is >> not suitably aligned. >> >> We need filter out O_DIRECT flag before read. > This is not very good changelog, I had to look closely at the source to > figure out why we may need this. > > Better description should say that the code which checks that the data > has been written correctly does not use aligned buffers, which may cause > a failure like the one above. > >> Signed-off-by: Ping Fang <pifang@redhat.com> >> --- >> testcases/kernel/io/ltp-aiodio/aiocp.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/testcases/kernel/io/ltp-aiodio/aiocp.c b/testcases/kernel/io/ltp-aiodio/aiocp.c >> index bc0e209b2..e4252d641 100644 >> --- a/testcases/kernel/io/ltp-aiodio/aiocp.c >> +++ b/testcases/kernel/io/ltp-aiodio/aiocp.c >> @@ -297,8 +297,8 @@ static void run(void) >> return; >> } >> >> - srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666); >> - dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666); >> + srcfd = SAFE_OPEN(srcname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); >> + dstfd = SAFE_OPEN(dstname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); > I guess that we can as well just remove the srcflags and keep just the > O_RDONLY since the srcflags are by definition either O_RDONLY or > O_DIRECT | O_RDONLY. > > I suppose that using scrflags and dstflags for anything else than the > filedescriptors passed to the async_run() is actually a mistake. > I agree with this. Probably we only need: srcfd = SAFE_OPEN(srcname, O_RDONLY, 0666); dstfd = SAFE_OPEN(dstname, O_RDONLY, 0666); >> reads = howmany(filesize, buffsize); >> >> -- >> 2.31.1 >> >> >> -- >> Mailing list info: https://lists.linux.it/listinfo/ltp Regards, Andrea Cervesato
On Thu, Feb 9, 2023 at 12:05 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > When aiocp executed with -f DIRECT will fail. > > > > <<<test_start>>> > > tag=AD049 stime=1675520824 > > cmdline="aiocp -b 8k -n 8 -f DIRECT" > > contacts="" > > analysis=exit > > <<<test_output>>> > > tst_test.c:1560: TINFO: Timeout per run is 0h 30m 30s > > aiocp.c:211: TINFO: Maximum AIO blocks: 65536 > > tst_device.c:585: TINFO: Use uevent strategy > > aiocp.c:250: TINFO: Fill srcfile.bin with random data > > aiocp.c:279: TINFO: Copy srcfile.bin -> dstfile.bin > > aiocp.c:291: TINFO: Comparing srcfile.bin with dstfile.bin > > aiocp.c:306: TBROK: read(3,0x7ffcd743abe0,4096) failed, returned -1: EINVAL (22) > > ... > > > > syscall read manual ERROR section said that: > > EINVAL fd is attached to an object which is unsuitable for reading; > > or the file was opened with the O_DIRECT flag, and either the address > > specified in buf, the value specified in count, or the file offset is > > not suitably aligned. > > > > We need filter out O_DIRECT flag before read. > > This is not very good changelog, I had to look closely at the source to > figure out why we may need this. > > Better description should say that the code which checks that the data > has been written correctly does not use aligned buffers, which may cause > a failure like the one above. +1, I also found it confusing (do we pass DIRECT as parameter just to ignore it?) > > > Signed-off-by: Ping Fang <pifang@redhat.com> > > --- > > testcases/kernel/io/ltp-aiodio/aiocp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/testcases/kernel/io/ltp-aiodio/aiocp.c b/testcases/kernel/io/ltp-aiodio/aiocp.c > > index bc0e209b2..e4252d641 100644 > > --- a/testcases/kernel/io/ltp-aiodio/aiocp.c > > +++ b/testcases/kernel/io/ltp-aiodio/aiocp.c > > @@ -297,8 +297,8 @@ static void run(void) > > return; > > } > > > > - srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666); > > - dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666); > > + srcfd = SAFE_OPEN(srcname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); > > + dstfd = SAFE_OPEN(dstname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); > > I guess that we can as well just remove the srcflags and keep just the > O_RDONLY since the srcflags are by definition either O_RDONLY or > O_DIRECT | O_RDONLY. > > I suppose that using scrflags and dstflags for anything else than the > filedescriptors passed to the async_run() is actually a mistake. > > > reads = howmany(filesize, buffsize); > > > > -- > > 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 >
On Thu, Feb 9, 2023 at 9:39 PM Jan Stancek <jstancek@redhat.com> wrote: > On Thu, Feb 9, 2023 at 12:05 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > > > Hi! > > > When aiocp executed with -f DIRECT will fail. > > > > > > <<<test_start>>> > > > tag=AD049 stime=1675520824 > > > cmdline="aiocp -b 8k -n 8 -f DIRECT" > > > contacts="" > > > analysis=exit > > > <<<test_output>>> > > > tst_test.c:1560: TINFO: Timeout per run is 0h 30m 30s > > > aiocp.c:211: TINFO: Maximum AIO blocks: 65536 > > > tst_device.c:585: TINFO: Use uevent strategy > > > aiocp.c:250: TINFO: Fill srcfile.bin with random data > > > aiocp.c:279: TINFO: Copy srcfile.bin -> dstfile.bin > > > aiocp.c:291: TINFO: Comparing srcfile.bin with dstfile.bin > > > aiocp.c:306: TBROK: read(3,0x7ffcd743abe0,4096) failed, returned -1: > EINVAL (22) > > > ... > > > > > > syscall read manual ERROR section said that: > > > EINVAL fd is attached to an object which is unsuitable for reading; > > > or the file was opened with the O_DIRECT flag, and either the address > > > specified in buf, the value specified in count, or the file offset is > > > not suitably aligned. > > > > > > We need filter out O_DIRECT flag before read. > > > > This is not very good changelog, I had to look closely at the source to > > figure out why we may need this. > > > > Better description should say that the code which checks that the data > > has been written correctly does not use aligned buffers, which may cause > > a failure like the one above. > > +1, I also found it confusing (do we pass DIRECT as parameter just to > ignore it?) > The data has been written successfully with DIRECT, the failure happens when reading it again with DIRECT flag. > > > > > > Signed-off-by: Ping Fang <pifang@redhat.com> > > > --- > > > testcases/kernel/io/ltp-aiodio/aiocp.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/testcases/kernel/io/ltp-aiodio/aiocp.c > b/testcases/kernel/io/ltp-aiodio/aiocp.c > > > index bc0e209b2..e4252d641 100644 > > > --- a/testcases/kernel/io/ltp-aiodio/aiocp.c > > > +++ b/testcases/kernel/io/ltp-aiodio/aiocp.c > > > @@ -297,8 +297,8 @@ static void run(void) > > > return; > > > } > > > > > > - srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666); > > > - dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666); > > > + srcfd = SAFE_OPEN(srcname, (srcflags & ~O_DIRECT) | O_RDONLY, > 0666); > > > + dstfd = SAFE_OPEN(dstname, (srcflags & ~O_DIRECT) | O_RDONLY, > 0666); > > > > I guess that we can as well just remove the srcflags and keep just the > > O_RDONLY since the srcflags are by definition either O_RDONLY or > > O_DIRECT | O_RDONLY. > > > > I suppose that using scrflags and dstflags for anything else than the > > filedescriptors passed to the async_run() is actually a mistake. > > > > > reads = howmany(filesize, buffsize); > > > > > > -- > > > 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 > > > >
diff --git a/testcases/kernel/io/ltp-aiodio/aiocp.c b/testcases/kernel/io/ltp-aiodio/aiocp.c index bc0e209b2..e4252d641 100644 --- a/testcases/kernel/io/ltp-aiodio/aiocp.c +++ b/testcases/kernel/io/ltp-aiodio/aiocp.c @@ -297,8 +297,8 @@ static void run(void) return; } - srcfd = SAFE_OPEN(srcname, srcflags | O_RDONLY, 0666); - dstfd = SAFE_OPEN(dstname, srcflags | O_RDONLY, 0666); + srcfd = SAFE_OPEN(srcname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); + dstfd = SAFE_OPEN(dstname, (srcflags & ~O_DIRECT) | O_RDONLY, 0666); reads = howmany(filesize, buffsize);
When aiocp executed with -f DIRECT will fail. <<<test_start>>> tag=AD049 stime=1675520824 cmdline="aiocp -b 8k -n 8 -f DIRECT" contacts="" analysis=exit <<<test_output>>> tst_test.c:1560: TINFO: Timeout per run is 0h 30m 30s aiocp.c:211: TINFO: Maximum AIO blocks: 65536 tst_device.c:585: TINFO: Use uevent strategy aiocp.c:250: TINFO: Fill srcfile.bin with random data aiocp.c:279: TINFO: Copy srcfile.bin -> dstfile.bin aiocp.c:291: TINFO: Comparing srcfile.bin with dstfile.bin aiocp.c:306: TBROK: read(3,0x7ffcd743abe0,4096) failed, returned -1: EINVAL (22) ... syscall read manual ERROR section said that: EINVAL fd is attached to an object which is unsuitable for reading; or the file was opened with the O_DIRECT flag, and either the address specified in buf, the value specified in count, or the file offset is not suitably aligned. We need filter out O_DIRECT flag before read. Signed-off-by: Ping Fang <pifang@redhat.com> --- testcases/kernel/io/ltp-aiodio/aiocp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)