Re: [LTP] [PATCH 1/3] lapi/fcntl.h: Fix O_DIRECT definition for various archs

[Petr Vorel <pvorel@suse.cz>]

> Hi!
> > O_DIRECT definitions differ a lot depending on architecture.
> > Because this can lead to problems (e.g. the generic value O_DIRECT
> > 040000 is O_DIRECTORY on powerpc, address that in lapi file.

> > NOTE: Deliberately use correct definitions also on less common archs
> > (maybe LTP even cannot be compiled on m68k or mips, but better to be
> > safe then sorry).  But the problem would IMHO be avoided if the fallback
> > O_DIRECT definition would be removed everywhere and tests just define
> > _GNU_SOURCE (no library code is using O_DIRECT atm).

> > Reported-by: NeilBrown <neilb@suse.de>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> >  include/lapi/fcntl.h | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)

> > diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
> > index 848ac7865..27da9f076 100644
> > --- a/include/lapi/fcntl.h
> > +++ b/include/lapi/fcntl.h
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0-or-later
> >  /*
> >   * Copyright (c) 2014 Cyril Hrubis <chrubis@suse.cz>
> > + * Copyright (c) Linux Test Project, 2014-2023
> >   */

> >  #ifndef LAPI_FCNTL_H__
> > @@ -11,7 +12,15 @@
> >  #include <sys/socket.h>

> >  #ifndef O_DIRECT
> > -# define O_DIRECT 040000
> > +# if defined(__mips__)
> > +#  define O_DIRECT 0100000
> > +# elif defined(__arm__) || defined(__aarch64__) || defined(__m68k__)
> > +#  define O_DIRECT 0200000
> > +# elif defined(__powerpc__) || defined(__powerpc64__)
> > +#  define O_DIRECT 0400000
> > +# else
> > +#  define O_DIRECT 040000
> > +# endif
> >  #endif

> I do not think that we should default to any value here, just do

> #error Define O_DIRECT for your architecture

> However I think that actually removing the O_DIRECT fallback and adding
> _GNU_SOURCE to tests that use O_DIRECT is more future proof solution.
> There is about 70 tests that use O_DIRECT and many of them define
> _GNU_SOURCE already.

I have no problem to remove O_DIRECT. The only question is what to do if we need
it in the library one day. Will we be ok that whole LTP becaume _GNU_SOURCE due
that?

> Actually I tried to remove the fallback from lapi/fcntl.h and recompiled
> LTP without any change, which is strange. Does that work for you as
> well?

I'm verifying it [1] in a patch, where I only defined _GNU_SOURCE in
testcases/kernel/fs/scsi/ltpscsi/scsimain.c and
testcases/kernel/fs/fsstress/global.h.

Kind regards,
Petr

[1] https://github.com/pevik/ltp/actions/runs/5401264765

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp