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.19-rc3+ compat: Remove refcount compat changes #7932

Merged
merged 1 commit into from Sep 26, 2018
Merged

[WIP] Linux 4.19-rc3+ compat: Remove refcount compat changes #7932

merged 1 commit into from Sep 26, 2018

Conversation

timschumi
Copy link
Contributor

@timschumi timschumi commented Sep 20, 2018

Motivation and Context

This resolved building ZFS for Linux 4.19-rc3 and above. The reason behind this is that some header file is using refcount_t now (introduced by torvalds/linux@59b5771), which is aliased to zfs_refcount_t due to the compatibility code.
This would fix Issue #7885

Description

This patch removes the compatibility changes for refcount_t and refcount_add and replaces all occurrences of them with their counterpart (i.e. zfs_refcount_t and zfs_refcount_add).

How Has This Been Tested?

I compiled and tested the modules with Linux 4.18.8 and Linux 4.19-rc4.
I haven't run the test suite yet (I'll do that when I have the time), but for now this patch is mainly there to fix building on newer kernel versions.

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)
  • Documentation (a change to man pages or other documentation)

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.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@timschumi
Copy link
Contributor Author

@philmmanjaro
Would you mind testing this to see if it resolves your issues?

torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Signed-off-by: Tim Schumacher <timschumi@gmx.de>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Sep 21, 2018
@ahrens
Copy link
Member

ahrens commented Sep 21, 2018

Assuming this is merged to ZoL, let's get it in illumos as well, otherwise this will cause lots of merge conflicts.

@philmmanjaro
Copy link

I've created a similar patch I currently use for v0.7.10 release. It compiles, however I didn't had time to test it yet, as I don't use zfs at all on my system.

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.

I had hoped to avoid this, as I sure you can tell by the preprocessor slight of hand, but it doesn't seem like that's going to be possible any longer. We should consider prefixing all of the refcount functions with zfs_ to prevent any future collisions and pushing that to OpenZFS.

Additionally, updating these zfs_refcount_* functions so they're layered on their optimized kernel counterparts wouldn't be a bad idea. I'm OK with doing this in multiple parts to first resolve the immediate build issue.

Copy link

@fpletz fpletz left a comment

Choose a reason for hiding this comment

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

Working fine on top of 0.8.0-rc1 with Linux 3.19-rc4.

@timschumi
Copy link
Contributor Author

@behlendorf
Thanks for looking at this. Should we prefix all the refcount functions and submit that to OpenZFS instead of this patch or should that be done at a later time (as a followup to this patch)? I don't feel comfortable submitting changes to OpenZFS directly, because I'm not able to test those changes (mainly due to lack of a test machine).

If we indeed go with this patch, should I fix the styling issue in the commit message or is that not worth pushing a new revision?

@behlendorf
Copy link
Contributor

@timschumi let's go with this minimal patch as is to solve the immediate problem. If you can follow up with a PR to add the zfs_ prefix that would be appreciated. Then I'm sure we can get some help to push both of these to OpenZFS in a form they're happy with.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 25, 2018
@behlendorf behlendorf merged commit c13060e into openzfs:master Sep 26, 2018
@timschumi timschumi deleted the patch/linux-4.19-compat branch September 27, 2018 01:47
@satmandu
Copy link
Contributor

satmandu commented Oct 8, 2018

Any chance of the 4.19 patch being ported back to the 0.7.x series for 0.7.12?

@tonyhutter tonyhutter added this to To do in 0.7.12 Oct 8, 2018
@tonyhutter
Copy link
Contributor

@satmandu I added it to the 0.7.12 patchlist (https://github.com/zfsonlinux/zfs/projects/20)

@tonyhutter tonyhutter moved this from To do to In progress in 0.7.12 Oct 26, 2018
@rincebrain rincebrain mentioned this pull request Oct 28, 2018
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 31, 2018
torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Reviewed-by: Franz Pletz <fpletz@fnordicwalking.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7885
Closes openzfs#7932
wichmannpas pushed a commit to wichmannpas/zfs that referenced this pull request Nov 3, 2018
torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Reviewed-by: Franz Pletz <fpletz@fnordicwalking.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7885 
Closes openzfs#7932
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 5, 2018
torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Reviewed-by: Franz Pletz <fpletz@fnordicwalking.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7885
Closes openzfs#7932
MorpheusTeam pushed a commit to Xyratex/lustre-stable that referenced this pull request Nov 6, 2018
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
tonyhutter pushed a commit that referenced this pull request Nov 13, 2018
torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Reviewed-by: Franz Pletz <fpletz@fnordicwalking.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes #7885
Closes #7932
GregorKopka pushed a commit to GregorKopka/zfs that referenced this pull request Jan 7, 2019
torvalds/linux@59b57717f ("blkcg: delay blkg destruction until
after writeback has finished") added a refcount_t to the blkcg
structure. Due to the refcount_t compatibility code, zfs_refcount_t
was used by mistake.

Resolve this by removing the compatibility code and replacing the
occurrences of refcount_t with zfs_refcount_t.

Reviewed-by: Franz Pletz <fpletz@fnordicwalking.de>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7885 
Closes openzfs#7932
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request May 16, 2019
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

Conflicts:
	config/lustre-build-zfs.m4
	lustre/osd-zfs/osd_internal.h
	lustre/osd-zfs/osd_object.c
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request May 28, 2019
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

Conflicts:
	config/lustre-build-zfs.m4
	lustre/osd-zfs/osd_internal.h
	lustre/osd-zfs/osd_object.c
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request Jun 22, 2019
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

Cherry-picked from 2.12.2-llnl
    89f8ee7 LU-11507 osd-zfs: Use zfs_refcount_add if available
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request Jun 3, 2020
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

Cherry-picked from 2.12.2-llnl
    89f8ee7 LU-11507 osd-zfs: Use zfs_refcount_add if available

Test-Parameters: forbuildonly
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request Jun 3, 2020
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Reviewed-on: https://review.whamcloud.com/33359
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Nathaniel Clark <nclark@whamcloud.com>
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

Cherry-pick-branch: 2.12.2-llnl
Cherry-pick-hash:89f8ee7

Test-Parameters: forbuildonly
ofaaland pushed a commit to ofaaland/lustre that referenced this pull request Jun 4, 2020
refcount_add was removed from ZFS master in:

    Linux 4.19-rc3+ compat: Remove refcount_t compat
    openzfs/zfs#7932

It is expected to be removed in zfs-0.7.12 as well.  Update Lustre
to use zfs_refcount_add if zfs supports it, and fall back to
refcount_add if not.

Cherry-pick-branch: 2.12.2-llnl
Cherry-pick-hash:89f8ee7

Test-Parameters: forbuildonly

Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Change-Id: Ib1b2ff13eb4ff8c56dd49a427b9827c6649ecd31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
No open projects
0.7.12
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

7 participants