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.

Sign up for GitHub

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

FreeBSD: Don't require zeroing new locks before init #10449

Merged
merged 1 commit into from
Jun 13, 2020
Merged

FreeBSD: Don't require zeroing new locks before init #10449

merged 1 commit into from
Jun 13, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 12, 2020 •
edited by ghost
Loading

Signed-off-by: Ryan Moeller ryan@iXsystems.com

Motivation and Context

This has not shown to be of use enough to justify the inconvenience.

Description

Remove the KASSERT in FreeBSD's mutex_init. While here, change the confusing and meaningless value of MUTEX_DEFAULT to 0 instead of 6.

How Has This Been Tested?

Built on FreeBSD, did some basic testing by creating a pool, checking out ZFS sources onto the pool, and building ZFS.

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.
  • I have run the ZFS Test Suite with this change applied.
  • All commit messages are properly formatted and contain Signed-off-by.

FreeBSD: Don't require zeroing new locks before init
f76f956 
This has not shown to be of use enough to justify the inconvenience.

Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
@ghost ghost added the Status: Code Review Needed Ready for review and testing label Jun 12, 2020
@ghost ghost requested a review from amotin June 12, 2020 15:29
amotin
amotin approved these changes Jun 12, 2020
View reviewed changes
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

I've never seen this assertion useful.

@codecov
Copy link

codecov bot commented Jun 12, 2020 •
edited
Loading

Codecov Report

Merging #10449 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10449      +/-   ##
==========================================
- Coverage   79.58%   79.42%   -0.16%     
==========================================
  Files         391      391              
  Lines      123872   123872              
==========================================
- Hits        98586    98388     -198     
- Misses      25286    25484     +198
Flag Coverage Δ
#kernel 79.97% <ø> (-0.06%) ⬇️
#user 65.41% <ø> (-0.71%) ⬇️
Impacted Files Coverage Δ
module/os/linux/spl/spl-zlib.c 55.35% <0.00%> (-28.58%) ⬇️
module/zfs/vdev_indirect.c 75.00% <0.00%> (-5.67%) ⬇️
module/zfs/bpobj.c 86.86% <0.00%> (-4.29%) ⬇️
cmd/ztest/ztest.c 76.58% <0.00%> (-3.56%) ⬇️
module/zfs/zio_compress.c 89.74% <0.00%> (-2.57%) ⬇️
module/zfs/dsl_synctask.c 92.40% <0.00%> (-2.54%) ⬇️
module/zfs/dsl_scan.c 84.58% <0.00%> (-1.36%) ⬇️
module/zfs/vdev_trim.c 95.34% <0.00%> (-0.76%) ⬇️
module/zfs/vdev_removal.c 96.80% <0.00%> (-0.69%) ⬇️
module/zfs/vdev_queue.c 94.62% <0.00%> (-0.60%) ⬇️
... and 49 more

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 e08b993...f76f956. Read the comment docs.

allanjude
allanjude approved these changes Jun 13, 2020
View reviewed changes
Copy link
Contributor

@allanjude allanjude left a comment

Choose a reason for hiding this comment

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

Reviewed by: allanjude@freebsd.org

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 13, 2020
@behlendorf behlendorf merged commit 499dccd into openzfs:master Jun 13, 2020
@ghost ghost deleted the allnewlocks branch June 13, 2020 20:24
lundman referenced this pull request in openzfsonosx/openzfs Jun 19, 2020
@lundman
FreeBSD: Don't require zeroing new locks before init
f7ca3a9 
This has not shown to be of use enough to justify the inconvenience.

Reviewed-by: Matt Macy <mmacy@FreeBSD.org>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10449
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
@jsai20
FreeBSD: Don't require zeroing new locks before init
4b7649e 
This has not shown to be of use enough to justify the inconvenience.

Reviewed-by: Matt Macy <mmacy@FreeBSD.org>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10449
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
FreeBSD: Don't require zeroing new locks before init
780f335 
This has not shown to be of use enough to justify the inconvenience.

Reviewed-by: Matt Macy <mmacy@FreeBSD.org>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes openzfs#10449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@allanjude @amotin @mattmacy @behlendorf