Update s390x syscall wrappers for RHEL-9.6#1426
Conversation
| /* arch/s390/include/asm/syscall_wrapper.h versions */ | ||
| # if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0) | ||
| # if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0) || \ | ||
| RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6) |
There was a problem hiding this comment.
Does it need to "#ifdef RHEL_RELEASE_CODE" before using it?
There was a problem hiding this comment.
Ah yes, that's right.
BTW, do you know if the preprocessor can handle this on a single line like:
#if defined(RHEL_RELEASE_CODE) && RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6)
w/o resorting to defining the macros in the non-RH case:
#ifndef RHEL_RELEASE_CODE
# define RHEL_RELEASE_CODE 1
# define RHEL_RELEASE_VERSION(x,y) 0
#endifIn other places in this repo, we defined HAVE_FEATURE macros based on up/down-stream versions and then switched around that instead. Do you have a preference?
There was a problem hiding this comment.
I think that should work. Either way works for me.
There was a problem hiding this comment.
Actually I'm not sure if that one-liner will work, i.e., I don't know whether the preprocessor stops evaluating the line when defined() returns false.
There was a problem hiding this comment.
Actually I'm not sure if that one-liner will work, i.e., I don't know whether the preprocessor stops evaluating the line when defined() returns false.
Seems like it doesn't:
$ cat test.c
#if defined(RHEL_RELEASE_CODE) && defined(RHEL_RELEASE_VERSION) && RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6)
#endif
$ gcc -c test.c -o test
test.c:1:109: error: missing binary operator before token "("
1 | #if defined(RHEL_RELEASE_CODE) && defined(RHEL_RELEASE_VERSION) && RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6)
| ^And my thought of trying to define RHEL_RELEASE_CODE / RHEL_RELEASE_VERSION wouldn't work too well either as it would only be accurate if the code checks RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION. Flip it around to <= and it will enable unintended blocks :(
So I think it will have to be the longer HAVE_xyz form.
There was a problem hiding this comment.
I don't see a shorter way to do it, how about (on top of this MR):
diff --git a/kmod/patch/kpatch-syscall.h b/kmod/patch/kpatch-syscall.h
index 8bd94f3a897f..058380bd88b3 100644
--- a/kmod/patch/kpatch-syscall.h
+++ b/kmod/patch/kpatch-syscall.h
@@ -91,9 +91,22 @@
#elif defined(CONFIG_S390)
+# if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0)
+# define KPATCH_SYSCALL_WRAPPERS_V2
+# else
+# define KPATCH_SYSCALL_WRAPPERS_V1
+# endif
+
+# if defined(RHEL_RELEASE_CODE)
+# if RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6)
+# define KPATCH_SYSCALL_WRAPPERS_V2
+# else
+# define KPATCH_SYSCALL_WRAPPERS_V1
+# endif
+
+
/* arch/s390/include/asm/syscall_wrapper.h versions */
-# if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 3, 0) || \
- RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(9, 6)
+#if defined(KPATCH_SYSCALL_WRAPPERS_V2)
#define __KPATCH_S390_SYS_STUBx(x, name, ...) \
long __s390_sys##name(struct pt_regs *regs); \
@@ -158,7 +171,7 @@
__diag_pop(); \
static inline long __kpatch_do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
-#endif /* LINUX_VERSION_CODE */
+#endif /* KPATCH_SYSCALL_WRAPPERS_V2 */
#elif defined(CONFIG_PPC64)
```
joe-lawrence
force-pushed
the
s390x-rhel-9.6-syscall
branch
from
524e071 to
2b76244
Compare
|
v2
|
|
Looks good, for readability can we do something like below (basically V1 first and add a comment to the '#else')? |
joe-lawrence
force-pushed
the
s390x-rhel-9.6-syscall
branch
from
2b76244 to
73fcbe0
Compare
|
v3:
|
|
I was also suggesting the V1 should come first, just so they're in logical order. |
RHEL-9.6 backported the upstream v6.3 s390x syscall updates, so add a distro-specific kernel version check around the correct definitions. Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
joe-lawrence
force-pushed
the
s390x-rhel-9.6-syscall
branch
from
73fcbe0 to
e42c799
Compare
|
v4:
|
2 participants
RHEL-9.6 backported upstream s390x syscall wrappers [1] that now need a distro-specific kernel version check.
[1] https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/5019/diffs?file=4c7281905bacd504a8205467bfd8e28ac241dbed#diff-content-4c7281905bacd504a8205467bfd8e28ac241dbed