Skip to content

Conversation

@timschumi
Copy link
Contributor

Motivation and Context

This brings refcount function naming in line with the zfs_refcount_add() method, which was
changed for Linux 4.19 compatibility. It should also prevent future collisions in case any other
functions get added.

Description

Prefixed every function definition and call with zfs_ and reformatted lines that were longer
than 80 chars after that.

How Has This Been Tested?

Compiled and tested with Linux 4.18.8 and 4.19.0-rc5. Ran zloop for about a hour and a half.

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.

@mschilli87 mschilli87 mentioned this pull request Sep 27, 2018
13 tasks
@codecov
Copy link

codecov bot commented Sep 27, 2018

Codecov Report

Merging #7963 into master will decrease coverage by 0.17%.
The diff coverage is 93.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7963      +/-   ##
==========================================
- Coverage   78.64%   78.47%   -0.18%     
==========================================
  Files         377      377              
  Lines      114014   114018       +4     
==========================================
- Hits        89668    89475     -193     
- Misses      24346    24543     +197
Flag Coverage Δ
#kernel 78.84% <93.87%> (-0.08%) ⬇️
#user 67.57% <87.2%> (-0.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d126145...9c81983. Read the comment docs.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Sep 27, 2018
@behlendorf behlendorf requested a review from ahrens September 27, 2018 19:14
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.

Looks good, thanks for the speedy follow up PR.

int64_t refcount_count(zfs_refcount_t *rc);
void zfs_refcount_create(zfs_refcount_t *rc);
void zfs_refcount_create_untracked(zfs_refcount_t *rc);
void zfs_refcount_create_tracked(zfs_refcount_t *rc);
Copy link
Member

Choose a reason for hiding this comment

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

While you're here, if you'd like to remove the argument names (throughout this header file), that would be good too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I can do that. It will have to wait until afternoon though (in about 7 hours).

@ahrens ahrens added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Sep 28, 2018
Recent changes in the Linux kernel made it necessary to prefix
the refcount_add() function with zfs_ due to a name collision.

To bring the other functions in line with that and to avoid future
collisions, prefix the other refcount functions as well.

Signed-off-by: Tim Schumacher <timschumi@gmx.de>
@behlendorf behlendorf merged commit 424fd7c into openzfs:master Oct 1, 2018
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 31, 2018
Recent changes in the Linux kernel made it necessary to prefix
the refcount_add() function with zfs_ due to a name collision.

To bring the other functions in line with that and to avoid future
collisions, prefix the other refcount functions as well.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7963
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 5, 2018
Recent changes in the Linux kernel made it necessary to prefix
the refcount_add() function with zfs_ due to a name collision.

To bring the other functions in line with that and to avoid future
collisions, prefix the other refcount functions as well.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7963
tonyhutter pushed a commit that referenced this pull request Nov 13, 2018
Recent changes in the Linux kernel made it necessary to prefix
the refcount_add() function with zfs_ due to a name collision.

To bring the other functions in line with that and to avoid future
collisions, prefix the other refcount functions as well.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes #7963
GregorKopka pushed a commit to GregorKopka/zfs that referenced this pull request Jan 7, 2019
Recent changes in the Linux kernel made it necessary to prefix
the refcount_add() function with zfs_ due to a name collision.

To bring the other functions in line with that and to avoid future
collisions, prefix the other refcount functions as well.

Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tim Schumacher <timschumi@gmx.de>
Closes openzfs#7963
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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants