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

FreeBSD: add support for KSTAT_TYPE_RAW #10836

Merged
merged 1 commit into from
Aug 30, 2020

Conversation

mattmacy
Copy link
Contributor

@mattmacy mattmacy commented Aug 27, 2020

The KSTAT_TYPE_RAW kstat type requires supporting a number of operations
that are really structured to fit in to Linux's seq_operations framework. The
somewhat awkward nature of the interface led me to leave out support for this
kstat type in the initial merge. This change fills in that initial gap.

KSTAT_TYPE_RAW is used by dbgmsg, spa state (currently private to Linux),
dbuf stats, vdev_raidz_bench, and fletcher_4_bench. Previously these were all
inaccessible to FreeBSD users.

Signed-off-by: Matt Macy [email protected]

Motivation and Context

Description

How Has This Been Tested?

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.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 28, 2020
@codecov
Copy link

codecov bot commented Aug 28, 2020

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff             @@
##           master   #10836       +/-   ##
===========================================
- Coverage   79.88%   65.77%   -14.11%     
===========================================
  Files         395      313       -82     
  Lines      125066   107848    -17218     
===========================================
- Hits        99905    70935    -28970     
- Misses      25161    36913    +11752     
Flag Coverage Δ
#kernel ?
#user 65.77% <ø> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
module/zfs/objlist.c 0.00% <0.00%> (-100.00%) ⬇️
module/zfs/pathname.c 0.00% <0.00%> (-100.00%) ⬇️
include/sys/dmu_redact.h 0.00% <0.00%> (-100.00%) ⬇️
include/sys/dmu_traverse.h 0.00% <0.00%> (-100.00%) ⬇️
module/lua/ltablib.c 2.34% <0.00%> (-95.32%) ⬇️
module/zfs/bqueue.c 0.00% <0.00%> (-94.45%) ⬇️
module/zfs/zfs_rlock.c 0.00% <0.00%> (-94.21%) ⬇️
module/zcommon/zfs_deleg.c 0.00% <0.00%> (-92.46%) ⬇️
module/zfs/dmu_diff.c 0.00% <0.00%> (-87.88%) ⬇️
module/zfs/zcp_set.c 0.00% <0.00%> (-87.10%) ⬇️
... and 235 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 62663fb...1cc1aef. Read the comment docs.

@tsoome
Copy link
Contributor

tsoome commented Aug 28, 2020

Can you add some more description... ?:)

A few kstats use KSTAT_TYPE_RAW to provide a string generated on
demand.  Implementing these as sysctls was punted until now.

Signed-off-by: Matt Macy <[email protected]>
@ghost ghost force-pushed the projects/kstat_raw_ops branch from 9599b14 to 1cc1aef Compare August 28, 2020 19:29
@mattmacy
Copy link
Contributor Author

Can you add some more description... ?:)

Done.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

At last! 👍

➜  ~ sysctl -n kstat.zfs.misc.fletcher_4_bench
implementation   native         byteswap
scalar           4718213052     4289191810
superscalar      6103790001     5190557091
superscalar4     4783991682     3966348686
sse2             7896467714     4572329578
ssse3            7896983221     7045094265
avx2             13080592052    11051043037
fastest          avx2           avx2

➜  ~ sysctl -n kstat.zfs.misc.vdev_raidz_bench
implementation   gen_p           gen_pq          gen_pqr         rec_p           rec_q           rec_r           rec_pq          rec_pr          rec_qr          rec_pqr
original         321256514       162625867       70124953        1181152392      210457911       26396112        62736139        15482992        15712710        10443469
scalar           1123886592      299494008       128174171       1120662593      336833534       251117481       160729139       133737740       102207369       67214395
sse2             2031949787      746825645       367459708       2029598955      964076290       674810510       493983566       420951379       269985820       181478781
ssse3            2032758781      746881016       368723915       2025218466      1083364478      826382250       599511721       489092621       363810509       275015085
avx2             3242070830      1064714074      619650935       3194125846      2001443592      1593867942      907346402       863817193       660207376       493693072
fastest          avx2            avx2            avx2            avx2            avx2            avx2            avx2            avx2            avx2            avx2

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 28, 2020
@behlendorf behlendorf merged commit d436de6 into openzfs:master Aug 30, 2020
behlendorf pushed a commit that referenced this pull request Aug 30, 2020
A few kstats use KSTAT_TYPE_RAW to provide a string generated on
demand.  Implementing these as sysctls was punted until now.

Reviewed by: Toomas Soome <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Matt Macy <[email protected]>
Closes #10836
@mattmacy mattmacy deleted the projects/kstat_raw_ops branch September 10, 2020 23:00
behlendorf pushed a commit that referenced this pull request Sep 21, 2020
Without this, the sysctl system calls will acquire a global lock before
invoking the handler.  This is noticeable in some situations when
running top(1).  The global lock is mostly vestigal but continues to see
some use and so contention is still a problem; until the default sense
of the MPSAFE flag changes, we have to annotate each and every handler.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Mark Johnston <[email protected]>
Closes #10836
behlendorf pushed a commit that referenced this pull request Oct 1, 2020
Without this, the sysctl system calls will acquire a global lock before
invoking the handler.  This is noticeable in some situations when
running top(1).  The global lock is mostly vestigal but continues to see
some use and so contention is still a problem; until the default sense
of the MPSAFE flag changes, we have to annotate each and every handler.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Mark Johnston <[email protected]>
Closes #10836
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
A few kstats use KSTAT_TYPE_RAW to provide a string generated on
demand.  Implementing these as sysctls was punted until now.

Reviewed by: Toomas Soome <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Matt Macy <[email protected]>
Closes openzfs#10836
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Without this, the sysctl system calls will acquire a global lock before
invoking the handler.  This is noticeable in some situations when
running top(1).  The global lock is mostly vestigal but continues to see
some use and so contention is still a problem; until the default sense
of the MPSAFE flag changes, we have to annotate each and every handler.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Mark Johnston <[email protected]>
Closes openzfs#10836
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
A few kstats use KSTAT_TYPE_RAW to provide a string generated on
demand.  Implementing these as sysctls was punted until now.

Reviewed by: Toomas Soome <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Matt Macy <[email protected]>
Closes openzfs#10836
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Without this, the sysctl system calls will acquire a global lock before
invoking the handler.  This is noticeable in some situations when
running top(1).  The global lock is mostly vestigal but continues to see
some use and so contention is still a problem; until the default sense
of the MPSAFE flag changes, we have to annotate each and every handler.

Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Ryan Moeller <[email protected]>
Signed-off-by: Mark Johnston <[email protected]>
Closes openzfs#10836
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.

4 participants