In-kernel build error: error: macro "__assign_str" passed 2 arguments, but takes just 1
#16475
Comments
|
Can you confirm whether it requires GPL license hack in META to reproduce to problem, or just copying files into kernel? In the former case, I think it's not that valuable to craft any fix to it. |
|
@Harry-Chen I have only tried myself with the META hack, but the vibe I got was that it would affect in-kernel builds as well. Personally, I don't care about either modes and don't think it should be possible to do (for reasons that are way outside the scope of this), thus wanting to remove |
|
In #16390 I simply removed the second parameter, it works for me and no error was caused (Although I accidentally corrected some places). |
|
@Pinghigh not enough, because the second arg is required in kernels before 6.10. Thus the need for the configure check to decide which one to use. |
|
@robn this configure check works for me (note it looks for 1ARG not 2ARGS like in your version): diff --git a/config/kernel-assign_str.m4 b/config/kernel-assign_str.m4
new file mode 100644
index 000000000..96159fae7
--- /dev/null
+++ b/config/kernel-assign_str.m4
@@ -0,0 +1,62 @@
+dnl #
+dnl # 6.10 kernel, check number of args of __assign_str() for trace:
+dnl
+dnl # 6.10+: one arg
+dnl # 6.9 and older: two args
+dnl #
+dnl # More specifically, this will test to see if __assign_str() takes one
+dnl # arg. If __assign_str() takes two args, or is not defined, then
+dnl # HAVE_1ARG_ASSIGN_STR will not be set.
+dnl #
+AC_DEFUN([ZFS_AC_KERNEL_1ARG_ASSIGN_STR], [
+ AC_MSG_CHECKING([whether __assign_str() has one arg])
+ ZFS_LINUX_TRY_COMPILE_HEADER([
+ #include <linux/module.h>
+ MODULE_LICENSE("$ZFS_META_LICENSE");
+
+ #define CREATE_TRACE_POINTS
+ #include "conftest.h"
+ ],[
+ trace_zfs_autoconf_event_one("1");
+ trace_zfs_autoconf_event_two("2");
+ ],[
+ AC_MSG_RESULT(yes)
+ AC_DEFINE(HAVE_1ARG_ASSIGN_STR, 1,
+ [__assign_str() has one arg])
+ ],[
+ AC_MSG_RESULT(no)
+ ],[
+ #if !defined(_CONFTEST_H) || defined(TRACE_HEADER_MULTI_READ)
+ #define _CONFTEST_H
+
+ #undef TRACE_SYSTEM
+ #define TRACE_SYSTEM zfs
+ #include <linux/tracepoint.h>
+
+ DECLARE_EVENT_CLASS(zfs_autoconf_event_class,
+ TP_PROTO(char *string),
+ TP_ARGS(string),
+ TP_STRUCT__entry(
+ __string(str, string)
+ ),
+ TP_fast_assign(
+ __assign_str(str);
+ ),
+ TP_printk("str = %s", __get_str(str))
+ );
+
+ #define DEFINE_AUTOCONF_EVENT(name) \
+ DEFINE_EVENT(zfs_autoconf_event_class, name, \
+ TP_PROTO(char * str), \
+ TP_ARGS(str))
+ DEFINE_AUTOCONF_EVENT(zfs_autoconf_event_one);
+ DEFINE_AUTOCONF_EVENT(zfs_autoconf_event_two);
+
+ #endif /* _CONFTEST_H */
+
+ #undef TRACE_INCLUDE_PATH
+ #define TRACE_INCLUDE_PATH .
+ #define TRACE_INCLUDE_FILE conftest
+ #include <trace/define_trace.h>
+ ])
+])
diff --git a/config/kernel.m4 b/config/kernel.m4
index 4d471358d..c3d0dcbed 100644
--- a/config/kernel.m4
+++ b/config/kernel.m4
@@ -328,6 +328,7 @@ AC_DEFUN([ZFS_AC_KERNEL_TEST_RESULT], [
ZFS_AC_KERNEL_SYNC_BDEV
ZFS_AC_KERNEL_MM_PAGE_SIZE
ZFS_AC_KERNEL_MM_PAGE_MAPPING
+ ZFS_AC_KERNEL_1ARG_ASSIGN_STR
case "$host_cpu" in
powerpc*)
ZFS_AC_KERNEL_CPU_HAS_FEATUREI couldn't get it to work with robn@ee94ce9c8 though: Hopefully someone with more tracepoint experience can wade though these errors. |
|
@tonyhutter Not looking through the code, just a guess, size 7 comes from As for the patch, do we really need a |
|
@wmmur Kernel versions are not typically used as evidence of whether some feature exists, because distros will backport commits in their own vendored kernels. It is necessary to perform actually compiling to test. But of course, it we can use more concise code snippets for testing availability, it would be good. |
|
FWIW I don't think tracing works with ZFS until there's some kind of change in the direction of license-compatibility (I've just verified against current zfs master + debian/sid 6.9.12 kernel): I got it to compile by doing this: vpsfreecz@f5045c2 - it seems to work like it should: Now the question is how to integrate it so it works with older kernels (and doesn't look stupid, that part seems the hardest for me :D) |
|
now I've got these two commits: vpsfreecz@e83f060 (with @tonyhutter as the author) and vpsfreecz@d59ddaf |
|
Was going to try 4.18, which is supposed to be supported, but that doesn't compile with more recent toolchain. Latest 4.19 + above patches + META change + tracing do seem to work together. So I should probably do a PR from this? |
__string field definition includes the source variable for a value of the string when the TP hits; in 6.10+ kernels, __assign_str() uses that to copy a value from src to the string, with older kernels, __assign_str still accepted src as a second parameter. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Pavel Snajdr <[email protected]> Co-authored-by: Tony Hutter <[email protected]> Closes #16475 Closes #16515
__string field definition includes the source variable for a value of the string when the TP hits; in 6.10+ kernels, __assign_str() uses that to copy a value from src to the string, with older kernels, __assign_str still accepted src as a second parameter. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Pavel Snajdr <[email protected]> Co-authored-by: Tony Hutter <[email protected]> Closes openzfs#16475 Closes openzfs#16515
__string field definition includes the source variable for a value of the string when the TP hits; in 6.10+ kernels, __assign_str() uses that to copy a value from src to the string, with older kernels, __assign_str still accepted src as a second parameter. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Pavel Snajdr <[email protected]> Co-authored-by: Tony Hutter <[email protected]> Closes openzfs#16475 Closes openzfs#16515
System information
Describe the problem you're observing
Build fails against kernel 6.10+ (in-kernel and "GPL hack" builds):
__assign_strin 6.10 dropped second argument in torvalds/linux@2c92ca849fcc.Most of the (probable) fix is robn/zfs@ee94ce9c8, but it needs configure support. I messed with it last night, but it didn't look like a simple check, because of the whole tracepoint sequencing thing. Probably lifting the guts of the
DECLARE_EVENT_CLASScheck and parameterising it to include__assign_str()is the "right" way to do it. I have no particular time or interest for this; feel free to grab the patch and finish it, and ask me if you like an assist.The text was updated successfully, but these errors were encountered: