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

Fix preemptible warning in aggsum_add() #7610

Merged
merged 1 commit into from Jun 7, 2018

Conversation

behlendorf
Copy link
Contributor

Description

In the new aggsum counters the CPU_SEQID macro should be surrounded by
kpreempt_disable)() and kpreempt_enable() calls to prevent a Linux
kernel BUG warning. The addsum_add() function use the cpuid to
minimize lock contention when selecting a bucket, after selection
the bucket is protected by a mutex and it is safe to reschedule the
process to a different processor at any time.

Motivation and Context

Issue #7609

How Has This Been Tested?

Locally built.

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.

In the new aggsum counters the CPU_SEQID macro should be surrounded by
kpreempt_disable)() and kpreempt_enable() calls to prevent a Linux
kernel BUG warning.  The addsum_add() function use the cpuid to
minimize lock contention when selecting a bucket, after selection
the bucket is protected by a mutex and it is safe to reschedule the
process to a different processor at any time.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#7609
Copy link
Contributor

@pcd1193182 pcd1193182 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 to me. This bug isn't correctness critical (which bucket we pick is ultimately a perf concern, we just want to spread the load), but it's good to squash the warning.

@prometheanfire
Copy link
Contributor

worked here (tested)

@behlendorf behlendorf merged commit 174bcd5 into openzfs:master Jun 7, 2018
@codecov
Copy link

codecov bot commented Jun 7, 2018

Codecov Report

Merging #7610 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7610      +/-   ##
==========================================
- Coverage   77.43%    77.4%   -0.03%     
==========================================
  Files         363      363              
  Lines      110505   110506       +1     
==========================================
- Hits        85567    85538      -29     
- Misses      24938    24968      +30
Flag Coverage Δ
#kernel 78.3% <100%> (+0.12%) ⬆️
#user 66.18% <100%> (-0.12%) ⬇️

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 b405837...76ace59. Read the comment docs.

@behlendorf behlendorf deleted the issue-7609 branch April 19, 2021 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants