[LTP] [PATCH v2 0/3] Remove O_DIRECT definition

[LTP] [PATCH v2 0/3] Remove O_DIRECT definition

[Petr Vorel]

Hi,

changes v1->v2:
Remove all O_DIRECT fallback definitions and add a note to define
_GNU_SOURCE to lapi header instead (v1 tried to add more archs, while
we could add current complete list it's error prone, using _GNU_SOURCE
is better).

Compilation tested:
https://github.com/pevik/ltp/actions/runs/5401264765

Kind regards,
Petr

Petr Vorel (3):
  lapi/fcntl.h: Remove O_DIRECT definition
  fsstress/global.h: Use _GNU_SOURCE
  ltpscsi/scsimain.c: Remove O_DIRECT fallback definition

 include/lapi/fcntl.h                        | 5 ++---
 testcases/kernel/fs/fsstress/global.h       | 8 +-------
 testcases/kernel/fs/scsi/ltpscsi/scsimain.c | 5 +----
 3 files changed, 4 insertions(+), 14 deletions(-)

-- 
2.40.1


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

[LTP] [PATCH v2 1/3] lapi/fcntl.h: Remove O_DIRECT definition

[Petr Vorel]

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. Therefore it's better just to remove
the definition in lapi and define _GNU_SOURCE in the tests which need
it (so far it's not needed for the library code).

Add this hint to the lapi header.

Reported-by: NeilBrown <neilb@suse.de>
Suggested-by: NeilBrown <neilb@suse.de>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/lapi/fcntl.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index 848ac7865..cb216e2dc 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__
@@ -10,9 +11,7 @@
 #include <fcntl.h>
 #include <sys/socket.h>
 
-#ifndef O_DIRECT
-# define O_DIRECT 040000
-#endif
+/* NOTE: #define _GNU_SOURCE if you need O_DIRECT in tests */
 
 #ifndef O_CLOEXEC
 # define O_CLOEXEC 02000000
-- 
2.40.1


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

[LTP] [PATCH v2 2/3] fsstress/global.h: Use _GNU_SOURCE

[Petr Vorel]

O_DIRECT definitions differ a lot depending on architecture,
therefore _GNU_SOURCE (which loads it) instead of fallback O_DIRECT
definition.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/fs/fsstress/global.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/testcases/kernel/fs/fsstress/global.h b/testcases/kernel/fs/fsstress/global.h
index 4ec382426..584af025d 100644
--- a/testcases/kernel/fs/fsstress/global.h
+++ b/testcases/kernel/fs/fsstress/global.h
@@ -33,7 +33,7 @@
 #ifndef GLOBAL_H
 #define GLOBAL_H
 
-/* xfs-specific includes */
+#define _GNU_SOURCE
 
 #if defined(NO_XFS)
 # include "xfscompat.h"
@@ -42,8 +42,6 @@
 # include <attr/attributes.h>
 #endif
 
-/* libc includes */
-
 #include <sys/stat.h>
 #include <sys/statvfs.h>
 #include <sys/time.h>
@@ -59,8 +57,4 @@
 #include <stdio.h>
 #include <unistd.h>
 
-#ifndef O_DIRECT
-#define O_DIRECT 040000
-#endif
-
 #endif
-- 
2.40.1


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

[LTP] [PATCH v2 3/3] ltpscsi/scsimain.c: Remove O_DIRECT fallback definition

[Petr Vorel]

To avoid potential problems with different O_DIRECT values across archs,
just replace the fallback definition with _GNU_SOURCE definition.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/fs/scsi/ltpscsi/scsimain.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/testcases/kernel/fs/scsi/ltpscsi/scsimain.c b/testcases/kernel/fs/scsi/ltpscsi/scsimain.c
index ae484e0f1..ce23ae1e7 100644
--- a/testcases/kernel/fs/scsi/ltpscsi/scsimain.c
+++ b/testcases/kernel/fs/scsi/ltpscsi/scsimain.c
@@ -28,6 +28,7 @@
 [0x12][   |lu][pg cde][res   ][al len][cntrl ]
 */
 
+#define _GNU_SOURCE
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdio.h>
@@ -53,10 +54,6 @@
 
 #define ME "scsimain: "
 
-#ifndef O_DIRECT
-#define O_DIRECT 040000
-#endif
-
 #define NUMERIC_SCAN_DEF 1	/* change to 0 to make alpha scan default */
 //static char * version_str = "0.21 20030513";
 
-- 
2.40.1


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

Re: [LTP] [PATCH v2 2/3] fsstress/global.h: Use _GNU_SOURCE

[Cyril Hrubis]

Hi!
> -/* xfs-specific includes */

Just a very minor nit, such cleanups should be in a separate patch.

Otherwise the rest of the changes looks good:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/3] fsstress/global.h: Use _GNU_SOURCE

[Petr Vorel]

Hi Cyril,

> Hi!
> > -/* xfs-specific includes */

> Just a very minor nit, such cleanups should be in a separate patch.
I'm sorry, I sometimes try to blance between separating changes and too many commits.
I can put this to separate commit.

> Otherwise the rest of the changes looks good:
Is that that this is just this patch. Please let me know if you're meant whole
patchset (it'd make sense, it's pretty simple).

> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 2/3] fsstress/global.h: Use _GNU_SOURCE

[Cyril Hrubis]

Hi!
> > Just a very minor nit, such cleanups should be in a separate patch.
> I'm sorry, I sometimes try to blance between separating changes and too many commits.
> I can put this to separate commit.

I tend to be picky in separating formatting changes from actuall fixes...

> > Otherwise the rest of the changes looks good:
> Is that that this is just this patch. Please let me know if you're meant whole
> patchset (it'd make sense, it's pretty simple).

That was for the whole patchset, sorry for not being clear.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/3] fsstress/global.h: Use _GNU_SOURCE

[Petr Vorel]

> Hi!
> > > Just a very minor nit, such cleanups should be in a separate patch.
> > I'm sorry, I sometimes try to blance between separating changes and too many commits.
> > I can put this to separate commit.

> I tend to be picky in separating formatting changes from actuall fixes...

I used to be, then relaxed a bit. But you're right, it makes sense.
Thus I separated this change to it's own commit.

> > > Otherwise the rest of the changes looks good:
> > Is that that this is just this patch. Please let me know if you're meant whole
> > patchset (it'd make sense, it's pretty simple).

> That was for the whole patchset, sorry for not being clear.

Thanks a lot, patchset merged (dared to add your RBT to the separated commit as
well.)

Kind regards,
Petr

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