Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux 6.10 compat: Remove second parameter of __assign_str() #16390

Closed
wants to merge 1 commit into from

Conversation

Pinghigh
Copy link

Remove second parameter of __assign_str()

Motivation and Context

The header files in /include/os/Linux/zfs/sys/ are behind some changes in Linux 6.10, which may cause some problems when compiling kernels with the zfs module built in.

Description

Simply removed the second parameter of __assign_str().

In Linux kernel commit 2c92ca849fcc6ee7d0c358e9959abc9f58661aea, the second parameter of __assign_str() was removed. As the instruction says, there is no need to pass the second parameter. But I didn't check if it would break anything to simply remove the second parameter.

How Has This Been Tested?

My custom Linux kernel (6.10.1) has compiled successfully with these changes and seems to be running well with no problems.

But I don't have the prerequisites to run the ZFS test suite. If there's a need for more in-depth testing, I'll try running it in a virtual machine.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

In Linux kernel commit 2c92ca849fcc6ee7d0c358e9959abc9f58661aea,
the second parameter of __assign_str() was removed. 
As the instruction says, there is no need to pass the second parameter.
But I didn't check if it would break anything to simply remove
the second parameter.

Signed-off-by: Pinghigh <[email protected]>
__entry->ds_object = db->db_objset->os_dsl_dataset ? \
db->db_objset->os_dsl_dataset->ds_object : 0; \
\
__entry->db_object = db->db.db_object; \
__entry->db_level = db->db_level; \
__entry->db_level = db->db_level;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to remove the \ here?

@tonyhutter
Copy link
Contributor

I tried building this PR against the 4.19 kernel and it worked fine. I then tried building it against 4.19, but with HAVE_DECLARE_EVENT_CLASS manually enabled in zfs_config.h:

/* DECLARE_EVENT_CLASS() is available */                                         
#define HAVE_DECLARE_EVENT_CLASS 1  

That gave me this error:

  CC [M]  /home/hutter/zfs/module/os/linux/zfs/zfs_dir.o
In file included from ./include/trace/trace_events.h:740,
                 from ./include/trace/define_trace.h:96,
                 from /home/hutter/zfs/include/os/linux/zfs/sys/trace_dbgmsg.h:80,
                 from /home/hutter/zfs/module/os/linux/zfs/trace.c:44:
/home/hutter/zfs/include/os/linux/zfs/sys/trace_dbgmsg.h:65:1: error: macro "__assign_str" requires 2 arguments, but only 1 given
   65 | );
      | ^~            
./include/trace/trace_events.h:673:9: note: macro "__assign_str" defined here
  673 | #define __assign_str(dst, src)                                          \
      |         ^~~~~~~~~~~~

So I think we need to add an automake check to look for what version of __assign_str() the kernel uses. You can use the configs/*.m4 scripts as templates for how to detect which kernel function version to use.

@Pinghigh
Copy link
Author

Sorry, my technical skills are insufficient to solve these problems temporarily, and due to being too busy to conduct in-depth research, this PR is temporarily closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants