[LTP] [PATCH v1] aiocp: Filter out O_DIRECT before read

[Jan Stancek]

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
>