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

[Li Wang]

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
> >
>
>

-- 
Regards,
Li Wang