-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Make the vfs.zfs.vdev.raidz_impl sysctl cross-platform #16980
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alan Somers <[email protected]> Sponsored by: ConnectWise
module/zfs/vdev.c
Outdated
@@ -6580,3 +6580,7 @@ ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, max_auto_ashift, | |||
param_set_max_auto_ashift, param_get_uint, ZMOD_RW, | |||
"Maximum ashift used when optimizing for logical -> physical sector " | |||
"size on new top-level vdevs"); | |||
|
|||
ZFS_MODULE_PARAM_CALL(zfs_vdev, zfs_vdev_, raidz_impl, | |||
param_set_raidz_impl, param_get_charp, ZMOD_RW, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems Linux get method is effectively missing. The param_get_charp()
will just return "todo" from zfs_vdev_raidz_impl
, that is never set. As I can see, vdev_raidz_impl_get()
is not called on Linux.
Signed-off-by: Alan Somers <[email protected]>
f27d84f
to
6a214e6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you are stil changing the behavior on Linus by using param_get_charp()
instead of vdev_raidz_impl_get()
before. I am not sure it is possible to drop the zfs_vdev_raidz_impl
variable completely with existing macros, but I don't think it is really needed here.
Yes, exactly. The existing macros seem to require that variable. |
But nobody should force you to use it, if as I understand in this case arbitrary string needs to be produced. |
Instead, define the FreeBSD and Linux sysctls separately. Signed-off-by: Alan Somers <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was an unexpected move.
Umm, what was unexpected? |
Splitting the tunable into two completely separate for FreeBSD and Linux. You just missed one function, but decided to turn 180 degrees and go other direction. |
Well, what would you like for me to do? |
I was thinking you replace |
Signed-off-by: Alan Somers [email protected]
Sponsored by: ConnectWise
Motivation and Context
It creates a
vfs.zfs.vdev.raidz_impl
sysctl on FreeBSD, so the implementation can be queried or changed.Description
Replace the Linux-specific
module_param_call
andMODULE_PARM_DESC
with the portableZFS_MODULE_PARAM_CALL
.How Has This Been Tested?
Tested on FreeBSD. Tried creating pools with every available raidz_impl testing. On Linux, compile-tested only.
Types of changes
Checklist:
Signed-off-by
.