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

Jump to bottom

Linux 4.8+ compatibility fix for vm stats #6528

Merged
merged 1 commit into from Aug 24, 2017
Merged

Linux 4.8+ compatibility fix for vm stats #6528

merged 1 commit into from Aug 24, 2017

Conversation

dbavatar
Copy link
Contributor

@dbavatar dbavatar commented Aug 17, 2017 •
edited

vm_node_stat must be used instead of vm_zone_stat. Unfortunately the
old code still compiles potentially leading to silent failure of
arc_evictable_memory()

AKAMAI: CR 3816601: Regression in zfs dropcache test

Signed-off-by: Debabrata Banerjee dbanerje@akamai.com

Description

New autoconf script to detect this

Motivation and Context

Upstream changed the variable used without making old code fail compilation
See 11fb998986a72aa7e997d96d63d52582a01228c5

How Has This Been Tested?

Tested with debug code that we're building against stat that works. Waiting for the buildbot to verify builds against everything :-)

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.

behlendorf
behlendorf requested changes Aug 18, 2017
View reviewed changes
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.

This explains quite a bit.

You'll want to rebase this against master to get a clean test run and we'll cherry-pick it back to the 0.7 release. @dinatale2 is working on extending the buildbot to properly test PRs against the 0.7-release branch but it's still a WIP.

We should add a small test case to the ZFS Test Suite to make sure we catch this kind of thing in the future. Something along the lines of what you originally posted in the issue would be enough.

config/kernel-vm_node_stat.m4 Outdated
(void) global_node_page_state(0);
],[
AC_MSG_RESULT(yes)
AC_DEFINE(ZFS_GLOBAL_PAGE_STATE, global_node_page_state, [using global_node_page_state()])
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: let's wrap this at 80 characters

@dbavatar
Copy link
Contributor Author

dbavatar commented Aug 18, 2017 •
edited

Unfortunately this solves very little in 7.x context, I believe there are 2-3 more issues that still need to be root caused there for it to work properly.

We should add a small test case to the ZFS Test Suite to make sure we catch this kind of thing in the future. Something along the lines of what you originally posted in the issue would be enough

We intend to port our common tests into the ZFS Test Suite, for example the drop cache test that caught the issue in 7.x. Unfortunately, even if we had it ready now it would fail on all branches.

@dbavatar dbavatar force-pushed the dbavatar/global_node_page_state branch from 208a114 to a1f5c3d Compare August 18, 2017 14:15
@dbavatar dbavatar changed the base branch from zfs-0.7-release to master August 18, 2017 14:16
@dbavatar dbavatar force-pushed the dbavatar/global_node_page_state branch from a1f5c3d to 3d3debb Compare August 18, 2017 14:17
@dbavatar dbavatar closed this Aug 18, 2017
@dbavatar dbavatar force-pushed the dbavatar/global_node_page_state branch from 3d3debb to 08de8c1 Compare August 18, 2017 14:25
@dbavatar dbavatar reopened this Aug 18, 2017
@behlendorf behlendorf requested a review from tuxoko August 21, 2017 16:18
tuxoko
tuxoko suggested changes Aug 22, 2017
View reviewed changes
config/kernel-vm_node_stat.m4 Outdated
(void) global_node_page_state(0);
],[
AC_MSG_RESULT(yes)
AC_DEFINE(ZFS_GLOBAL_PAGE_STATE, global_node_page_state,
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the marcro definition in a .h file, otherwise it might be hard to find it.

@dbavatar dbavatar force-pushed the dbavatar/global_node_page_state branch 2 times, most recently from 93eff83 to 8c18625 Compare August 23, 2017 20:22
@dbavatar
Linux 4.8+ compatibility fix for vm stats
effe86c 
vm_node_stat must be used instead of vm_zone_stat. Unfortunately the
old code still compiles potentially leading to silent failure of
arc_evictable_memory()

AKAMAI: CR 3816601: Regression in zfs dropcache test

Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
@dbavatar dbavatar force-pushed the dbavatar/global_node_page_state branch from 8c18625 to effe86c Compare August 24, 2017 14:09
@behlendorf behlendorf merged commit 2209e40 into openzfs:master Aug 24, 2017
@behlendorf behlendorf added this to TODO in 0.7.2 Aug 24, 2017
tonyhutter pushed a commit that referenced this pull request Sep 13, 2017
@dbavatar @tonyhutter
Linux 4.8+ compatibility fix for vm stats
8995072 
vm_node_stat must be used instead of vm_zone_stat. Unfortunately the
old code still compiles potentially leading to silent failure of
arc_evictable_memory()

AKAMAI: CR 3816601: Regression in zfs dropcache test

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Chunwei Chen <tuxoko@gmail.com>
Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
Closes #6528
@tonyhutter tonyhutter moved this from TODO to Done in 0.7.2 Sep 13, 2017
@chrisrd chrisrd mentioned this pull request Sep 14, 2017
Merged
13 tasks
Fabian-Gruenbichler pushed a commit to Fabian-Gruenbichler/zfs that referenced this pull request Sep 29, 2017
@dbavatar @Fabian-Gruenbichler
Linux 4.8+ compatibility fix for vm stats
307f174 
vm_node_stat must be used instead of vm_zone_stat. Unfortunately the
old code still compiles potentially leading to silent failure of
arc_evictable_memory()

AKAMAI: CR 3816601: Regression in zfs dropcache test

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Chunwei Chen <tuxoko@gmail.com>
Signed-off-by: Debabrata Banerjee <dbanerje@akamai.com>
Closes openzfs#6528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tuxoko tuxoko tuxoko approved these changes

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
0.7.2
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dbavatar @tuxoko @behlendorf