Prevent metaslab_sync panic due to spa_final_dirty_txg #9231

pcd1193182 · 2019-08-29T00:15:51Z

This change has a cherry-picked commit from #9209 as well, since it will be integrated after that commit. It also reverts 'ZTS: Temporarily disable several upgrade tests', since it fixes the issues that was intended to work around.

Motivation and Context

See #9186

Description

If a pool enables the SPACEMAP_HISTOGRAM feature shortly before being
exported, we can enter a situation that causes a kernel panic. Any metaslabs
that are loaded during the final dirty txg and haven't already been condensed
will cause metaslab_sync to proceed after the final dirty txg so that the
condense can be performed, which there are assertions to prevent. Because of
the nature of this issue, there are a number of ways we can enter this
state. Rather than try to prevent each of them one by one, potentially missing
some edge cases, we instead cut it off at the point of intersection; by
preventing metaslab_sync from proceeding if it would only do so to perform a
condense and we're past the final dirty txg, we preserve the utility of the
existing asserts while preventing this particular issue.

How Has This Been Tested?

zfs-test and zloop

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.

If a pool enables the SPACEMAP_HISTOGRAM feature shortly before being exported, we can enter a situation that causes a kernel panic. Any metaslabs that are loaded during the final dirty txg and haven't already been condensed will cause metaslab_sync to proceed after the final dirty txg so that the condense can be performed, which there are assertions to prevent. Because of the nature of this issue, there are a number of ways we can enter this state. Rather than try to prevent each of them one by one, potentially missing some edge cases, we instead cut it off at the point of intersection; by preventing metaslab_sync from proceeding if it would only do so to perform a condense and we're past the final dirty txg, we preserve the utility of the existing asserts while preventing this particular issue. Signed-off-by: Paul Dagnelie <pcd@delphix.com>

module/zfs/metaslab.c

behlendorf

Can you split the ZTS revert in to a separate commit in this PR. There's still some discussion going on in #9209 regarding the best way to handle that case. Until both cases are resolved we can't re-enable the tests, but that shouldn't prevent us from integrating this fix.

Signed-off-by: Paul Dagnelie <pcd@delphix.com>

ahrens · 2019-08-29T19:06:37Z

I think that we actually don't need #9209. This change should be sufficient to fix the bug and re-enable the tests.

behlendorf · 2019-08-29T20:02:32Z

I'll make sure this gets run through the CI a few times to confirm that's the case.

codecov · 2019-08-29T21:46:03Z

Codecov Report

Merging #9231 into master will decrease coverage by 2.26%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #9231      +/-   ##
==========================================
- Coverage   79.08%   76.82%   -2.27%     
==========================================
  Files         400      373      -27     
  Lines      122034   118922    -3112     
==========================================
- Hits        96516    91365    -5151     
- Misses      25518    27557    +2039

Flag	Coverage Δ
#kernel	`76.03% <100%> (-3.68%)`	⬇️
#user	`66.07% <50%> (-0.93%)`	⬇️

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 f66ad58...2d5d5df. Read the comment docs.

behlendorf · 2019-08-30T16:25:37Z

This looks good. The issue hasn't been reproduced by the CI after over 15 full ZTS runs.

If a pool enables the SPACEMAP_HISTOGRAM feature shortly before being exported, we can enter a situation that causes a kernel panic. Any metaslabs that are loaded during the final dirty txg and haven't already been condensed will cause metaslab_sync to proceed after the final dirty txg so that the condense can be performed, which there are assertions to prevent. Because of the nature of this issue, there are a number of ways we can enter this state. Rather than try to prevent each of them one by one, potentially missing some edge cases, we instead cut it off at the point of intersection; by preventing metaslab_sync from proceeding if it would only do so to perform a condense and we're past the final dirty txg, we preserve the utility of the existing asserts while preventing this particular issue. Reviewed-by: Matt Ahrens <matt@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#9185 Closes openzfs#9186 Closes openzfs#9231 Closes openzfs#9253

If a pool enables the SPACEMAP_HISTOGRAM feature shortly before being exported, we can enter a situation that causes a kernel panic. Any metaslabs that are loaded during the final dirty txg and haven't already been condensed will cause metaslab_sync to proceed after the final dirty txg so that the condense can be performed, which there are assertions to prevent. Because of the nature of this issue, there are a number of ways we can enter this state. Rather than try to prevent each of them one by one, potentially missing some edge cases, we instead cut it off at the point of intersection; by preventing metaslab_sync from proceeding if it would only do so to perform a condense and we're past the final dirty txg, we preserve the utility of the existing asserts while preventing this particular issue. Reviewed-by: Matt Ahrens <matt@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes #9185 Closes #9186 Closes #9231 Closes #9253

If a pool enables the SPACEMAP_HISTOGRAM feature shortly before being exported, we can enter a situation that causes a kernel panic. Any metaslabs that are loaded during the final dirty txg and haven't already been condensed will cause metaslab_sync to proceed after the final dirty txg so that the condense can be performed, which there are assertions to prevent. Because of the nature of this issue, there are a number of ways we can enter this state. Rather than try to prevent each of them one by one, potentially missing some edge cases, we instead cut it off at the point of intersection; by preventing metaslab_sync from proceeding if it would only do so to perform a condense and we're past the final dirty txg, we preserve the utility of the existing asserts while preventing this particular issue. Reviewed-by: Matt Ahrens <matt@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Paul Dagnelie <pcd@delphix.com> Closes openzfs#9185 Closes openzfs#9186 Closes openzfs#9231 Closes openzfs#9253

pcd1193182 added the Status: Code Review Needed Ready for review and testing label Aug 29, 2019

pcd1193182 requested review from behlendorf and ahrens August 29, 2019 00:15

pcd1193182 changed the title ~~Condense~~ Prevent metaslab_sync final_dirty_txg panic Aug 29, 2019

pcd1193182 changed the title ~~Prevent metaslab_sync final_dirty_txg panic~~ @pcd1193182 Prevent metaslab_sync panic due to spa_final_dirty_txg Aug 29, 2019

ikozhukhov mentioned this pull request Aug 29, 2019

panic with changes 'Cap metaslab memory usage' #9253

Closed

ahrens changed the title ~~@pcd1193182 Prevent metaslab_sync panic due to spa_final_dirty_txg~~ Prevent metaslab_sync panic due to spa_final_dirty_txg Aug 29, 2019

ahrens approved these changes Aug 29, 2019

View reviewed changes

module/zfs/metaslab.c Outdated Show resolved Hide resolved

behlendorf approved these changes Aug 29, 2019

View reviewed changes

Matt comment feedback

2d5d5df

Signed-off-by: Paul Dagnelie <pcd@delphix.com>

pcd1193182 force-pushed the condense branch from f0676d2 to 2d5d5df Compare August 29, 2019 19:05

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 29, 2019

behlendorf merged commit 475aa97 into openzfs:master Aug 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent metaslab_sync panic due to spa_final_dirty_txg #9231

Prevent metaslab_sync panic due to spa_final_dirty_txg #9231

pcd1193182 commented Aug 29, 2019

behlendorf left a comment

ahrens commented Aug 29, 2019

behlendorf commented Aug 29, 2019

codecov bot commented Aug 29, 2019 •

edited

Loading

behlendorf commented Aug 30, 2019

Prevent metaslab_sync panic due to spa_final_dirty_txg #9231

Prevent metaslab_sync panic due to spa_final_dirty_txg #9231

Conversation

pcd1193182 commented Aug 29, 2019

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

behlendorf left a comment

Choose a reason for hiding this comment

ahrens commented Aug 29, 2019

behlendorf commented Aug 29, 2019

codecov bot commented Aug 29, 2019 • edited Loading

Codecov Report

behlendorf commented Aug 30, 2019

codecov bot commented Aug 29, 2019 •

edited

Loading