FreeBSD: Implement sysctl for fletcher4 impl #11270

ghost · 2020-12-02T21:51:40Z

Motivation and Context

There is a tunable to select the fletcher 4 checksum implementation on
Linux but it was not present in FreeBSD.

Description

Implement the sysctl handler for FreeBSD and create a new macro
ZFS_MODULE_VIRTUAL_PARAM_CALL to provide the tunable on both platforms.

ZFS_MODULE_VIRTUAL_PARAM_CALL is like ZFS_MODULE_PARAM_CALL except it
does not try to pass the address of a variable to module_param_call on
Linux, so there does not need to be a variable with a name matching the
tunable.

How Has This Been Tested?

FreeBSD-13_0-CURRENT-9e082d278b9 ➜  ZoF git:(fletcher-4-impl) ✗ sysctl vfs.zfs.fletcher_4_impl
vfs.zfs.fletcher_4_impl: [fastest] scalar superscalar superscalar4 sse2 ssse3 avx2
FreeBSD-13_0-CURRENT-9e082d278b9 ➜  ZoF git:(fletcher-4-impl) ✗ sudo sysctl vfs.zfs.fletcher_4_impl=scalar
vfs.zfs.fletcher_4_impl: [fastest] scalar superscalar superscalar4 sse2 ssse3 avx2  -> fastest [scalar] superscalar superscalar4 sse2 ssse3 avx2
FreeBSD-13_0-CURRENT-9e082d278b9 ➜  ZoF git:(fletcher-4-impl) ✗ sudo sysctl vfs.zfs.fletcher_4_impl
vfs.zfs.fletcher_4_impl: fastest [scalar] superscalar superscalar4 sse2 ssse3 avx2
FreeBSD-13_0-CURRENT-9e082d278b9 ➜  ZoF git:(fletcher-4-impl) ✗ sudo sysctl vfs.zfs.fletcher_4_impl=fastest
vfs.zfs.fletcher_4_impl: fastest [scalar] superscalar superscalar4 sse2 ssse3 avx2  -> [fastest] scalar superscalar superscalar4 sse2 ssse3 avx2

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 OpenZFS 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.

ghost · 2020-12-04T19:49:52Z

Bah, forgot to fix the test -_-

ghost · 2020-12-07T22:08:04Z

Add ZFS_MODULE_VIRTUAL_PARAM_CALL() to fix the build on Linux, since ZFS_MODULE_PARAM_CALL() requires a variable with a particular name to actually exist so its address can be passed to module_param_call().

ghost · 2020-12-08T16:22:21Z

The arc_summary failure brings to my attention some issues with the Python utilities that should be dealt with separately.

ghost · 2020-12-08T18:21:10Z

Weird, I don't find the tunable on Linux with this patch:

$ ls /sys/module/zfs/parameters/*impl
/sys/module/zfs/parameters/zfs_vdev_raidz_impl

behlendorf · 2020-12-08T19:42:23Z

On Linux the fletcher4 tunable appears in the zcommon and not zfs directory.

ls /sys/module/zcommon/parameters/*impl
/sys/module/zcommon/parameters/zfs_fletcher_4_impl

ghost · 2020-12-08T20:18:04Z

Ok great that explains why arc_summary was always fine on Linux :)

There is a tunable to select the fletcher 4 checksum implementation on Linux but it was not present in FreeBSD. Implement the sysctl handler for FreeBSD and use ZFS_MODULE_PARAM_CALL to provide the tunable on both platforms. Signed-off-by: Ryan Moeller <ryan@iXsystems.com>

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Signed-off-by: Ryan Moeller <ryan@iXsystems.com>

ghost · 2020-12-09T16:39:13Z

I guess I pushed at a bad time, pushed again to retrigger the bots.

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11270

There is a tunable to select the fletcher 4 checksum implementation on Linux but it was not present in FreeBSD. Implement the sysctl handler for FreeBSD and use ZFS_MODULE_PARAM_CALL to provide the tunable on both platforms. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

There is a tunable to select the fletcher 4 checksum implementation on Linux but it was not present in FreeBSD. Implement the sysctl handler for FreeBSD and use ZFS_MODULE_PARAM_CALL to provide the tunable on both platforms. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11270

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11270

There is a tunable to select the fletcher 4 checksum implementation on Linux but it was not present in FreeBSD. Implement the sysctl handler for FreeBSD and use ZFS_MODULE_PARAM_CALL to provide the tunable on both platforms. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

There is a tunable to select the fletcher 4 checksum implementation on Linux but it was not present in FreeBSD. Implement the sysctl handler for FreeBSD and use ZFS_MODULE_PARAM_CALL to provide the tunable on both platforms. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

Some tunables shown by arc_summary3 have string values that may exceed the normal line length, leaving a negative offset between the name and value fields. The negative space is of course not valid and Python rightly barfs up an exception traceback. Handle an overflowing value field width by ignoring the line length and separating the name from the value by a single space instead. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11270

ghost added Status: Code Review Needed Ready for review and testing Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Dec 2, 2020

ghost force-pushed the fletcher-4-impl branch from 21012d7 to ab3de2b Compare December 4, 2020 19:49

ghost force-pushed the fletcher-4-impl branch from ab3de2b to 323a69b Compare December 7, 2020 22:05

ghost added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Dec 7, 2020

ghost added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Dec 8, 2020

ghost force-pushed the fletcher-4-impl branch from 323a69b to 4c405a9 Compare December 8, 2020 20:28

ghost force-pushed the fletcher-4-impl branch from 4c405a9 to 5289790 Compare December 9, 2020 16:38

ghost added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Dec 9, 2020

behlendorf approved these changes Dec 10, 2020

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 10, 2020

behlendorf closed this in 439dc03 Dec 11, 2020

ghost deleted the fletcher-4-impl branch December 11, 2020 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FreeBSD: Implement sysctl for fletcher4 impl #11270

FreeBSD: Implement sysctl for fletcher4 impl #11270

ghost commented Dec 2, 2020 •

edited by ghost

ghost commented Dec 4, 2020

ghost commented Dec 7, 2020

ghost commented Dec 8, 2020

ghost commented Dec 8, 2020

behlendorf commented Dec 8, 2020

ghost commented Dec 8, 2020

ghost commented Dec 9, 2020

FreeBSD: Implement sysctl for fletcher4 impl #11270

FreeBSD: Implement sysctl for fletcher4 impl #11270

Conversation

ghost commented Dec 2, 2020 • edited by ghost

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

ghost commented Dec 4, 2020

ghost commented Dec 7, 2020

ghost commented Dec 8, 2020

ghost commented Dec 8, 2020

behlendorf commented Dec 8, 2020

ghost commented Dec 8, 2020

ghost commented Dec 9, 2020

ghost commented Dec 2, 2020 •

edited by ghost