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

[WIP] linux 4.11 compat: avoid refcount_t name conflict #5842

Merged
merged 1 commit into from
Mar 1, 2017

Conversation

ofaaland
Copy link
Contributor

@ofaaland ofaaland commented Feb 27, 2017

Description

Rename the ZFS type refcount_t to zfs_refcount_t.

Rather than touching all the ZFS code that uses refcount_t, use a preprocessor
macro to rename the type at compile time to zfs_refcount_t. The ZFS code
never refers to the linux type with the conflicting name so this does not break
anything.

Similarly rename refcount_add() which also conflicts.

Motivation and Context

Linux 4.11 introduces a new type, refcount_t, which conflicts with the
type of the same name defined within ZFS. The build fails when building
against that kernel or later.

Issue #5823

How Has This Been Tested?

Built and ran ztest on my desktop.

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)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • [] All new and existing tests passed.
  • Change has been approved by a ZFS on Linux member.

@ofaaland ofaaland added the Status: Work in Progress Not yet ready for general review label Feb 27, 2017
Linux 4.11 introduces a new type, refcount_t, which conflicts with the
type of the same name defined within ZFS.

Rename the ZFS type zfs_refcount_t.  Within the ZFS code, use a macro to
cause references to refcount_t to be changed to zfs_refcount_t at
compile time.  This reduces conflicts when later landing OpenZFS
patches.

Fixes openzfs#5823

Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

LGTM. I agree a minimal fix is best here to avoid potential future conflicts.

@behlendorf behlendorf added Type: Building Indicates an issue related to building binaries and removed Status: Work in Progress Not yet ready for general review labels Feb 28, 2017
@tuxoko
Copy link
Contributor

tuxoko commented Feb 28, 2017

To be honest, I don't like this kind of macro game very much. If you're not careful about the include order, it's very easy to accidentally modify the kernel header and cause some strange thing.

We did this kind of thing with kmem_cache thing, but kmem_cache object and function are rarely used inside kernel header files so it's arguably not a big deal. However, you can't say the same thing for refcount_t, it's definitely going to be used in a lot of header files.

@behlendorf
Copy link
Contributor

The long term plan discussed at the OpenZFS dev summit would be to add a prefix, maybe zk_ to all these interfaces to prevent a name conflict across platforms. So this should be a medium term build fix. I'd rather not have to play these macro games either but for the moment it's probably the least disruptive build fix.

@behlendorf behlendorf merged commit 4859fe7 into openzfs:master Mar 1, 2017
@behlendorf behlendorf added this to TODO in 0.6.5.10 Mar 1, 2017
@behlendorf behlendorf moved this from TODO to Done in 0.6.5.10 Mar 1, 2017
@behlendorf behlendorf moved this from Done to TODO in 0.6.5.10 Mar 1, 2017
@ofaaland ofaaland deleted the b_refcount_t branch March 1, 2017 18:11
l1k pushed a commit to l1k/zfs that referenced this pull request Apr 17, 2017
Linux 4.11 introduces a new type, refcount_t, which conflicts with the
type of the same name defined within ZFS.

Rename the ZFS type zfs_refcount_t.  Within the ZFS code, use a macro to
cause references to refcount_t to be changed to zfs_refcount_t at
compile time.  This reduces conflicts when later landing OpenZFS
patches.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#5823
Closes openzfs#5842
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 18, 2017
Linux 4.11 introduces a new type, refcount_t, which conflicts with the
type of the same name defined within ZFS.

Rename the ZFS type zfs_refcount_t.  Within the ZFS code, use a macro to
cause references to refcount_t to be changed to zfs_refcount_t at
compile time.  This reduces conflicts when later landing OpenZFS
patches.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes openzfs#5823
Closes openzfs#5842
@behlendorf behlendorf moved this from TODO to Done in 0.6.5.10 May 25, 2017
tonyhutter pushed a commit that referenced this pull request Jun 9, 2017
Linux 4.11 introduces a new type, refcount_t, which conflicts with the
type of the same name defined within ZFS.

Rename the ZFS type zfs_refcount_t.  Within the ZFS code, use a macro to
cause references to refcount_t to be changed to zfs_refcount_t at
compile time.  This reduces conflicts when later landing OpenZFS
patches.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Olaf Faaland <faaland1@llnl.gov>
Closes #5823
Closes #5842
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Building Indicates an issue related to building binaries
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants