Console View
|
Tags: Architectures Platforms default |
|
| Architectures | Platforms | default | |||||||||||||||||||||
|
|
|
|
|||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Fix double const qualifier declarations Some header files define structures like this one: typedef const struct zio_checksum_info { /* ... */ const char *ci_name; } zio_abd_checksum_func_t; So we can use `zio_abd_checksum_func_t` for const declarations now. It's not needed that we use the `const` qualifier again like this: `const zio_abd_checksum_func_t *varname;` This patch solves the double const qualifiers, which were found by smatch. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13961 part 1/1 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Fix double const qualifier declarations Some header files define structures like this one: typedef const struct zio_checksum_info { /* ... */ const char *ci_name; } zio_abd_checksum_func_t; So we can use `zio_abd_checksum_func_t` for const declarations now. It's not needed that we use the `const` qualifier again like this: `const zio_abd_checksum_func_t *varname;` This patch solves the double const qualifiers, which were found by smatch. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13961 part 1/1 |
|||||||||||||||||||||||
|
Andrew Innes
andrew.c12@gmail.com |
|
|
|
||||||||||||||||||||
|
Windows: use common zoneid_t Signed-off-by: Andrew Innes <andrew.c12@gmail.com> Pull-request: #13960 part 286/286 |
|||||||||||||||||||||||
|
Andrew Innes
andrew.c12@gmail.com |
|
|
|
||||||||||||||||||||
|
add readme header (#140) Signed-off-by: Andrew Innes <andrew.c12@gmail.com> Pull-request: #13960 part 284/284 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
ZED: Enforce format specifier correctness While experimenting with ways to reduce false positives in static analyzers, I discovered that we could get the compiler to enforce correctness for format specifiers by annotating function prototypes. This patch only applies to ZED, since we are not annotating things that affect the kernel, such as zfs_panic_recover(). Unfortunately, we do not have inttypes.h inside the kernel, so we need to hard code format specifiers if we annotate things that affect the kernel. However, GCC has this interesting behavior on amd64 where if you use %llu with uint64_t, it will complain that uint64_t is unsigned long int and suggests %lu, but if you actually use %lu, it will complain that uint64_t is unsigned long long int. We could go through the trouble of defining PRIu64 and changing it to %llu or %lu depending on the architecture, like is done in inttypes.h, but upon inspection, it appears that the Linux kernel treats %llu and %lu the same, no matter whether the system is 64-bit or 32-bit, so getting it right is a moot point. The FreeBSD behavior was not checked. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13956 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
ZED: Enforce format specifier correctness While experimenting with ways to reduce false positives in static analyzers, I discovered that we could get the compiler to enforce correctness for format specifiers by annotating function prototypes. This patch only applies to ZED, since we are not annotating things that affect the kernel, such as zfs_panic_recover(). Unfortunately, we do not have inttypes.h inside the kernel, so we need to hard code format specifiers if we annotate things that affect the kernel. However, GCC has this interesting behavior on amd64 where if you use %llu with uint64_t, it will complain that uint64_t is unsigned long int and suggests %lu, but if you actually use %lu, it will complain that uint64_t is unsigned long long int. We could go through the trouble of defining PRIu64 and changing it to %llu or %lu depending on the architecture, like is done in inttypes.h, but upon inspection, it appears that the Linux kernel treats %llu and %lu the same, no matter whether the system is 64-bit or 32-bit, so getting it right is a moot point. The FreeBSD behavior was not checked. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13956 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
ZED: Enforce format specifier correctness While experimenting with ways to reduce false positives in static analyzers, I discovered that we could get the compiler to enforce correctness for format specifiers by annotating function prototypes. This patch only applies to ZED, since we are not annotating things that affect the kernel, such as zfs_panic_recover(). Unfortunately, we do not have inttypes.h inside the kernel, so we need to hard code format specifiers if we annotate things that affect the kernel. However, GCC has this interesting behavior on amd64 where if you use %llu with uint64_t, it will complain that uint64_t is unsigned long long int and suggests %lu, but if you actually use %lu, it will complain that uint64_t is unsigned long int. We could go through the trouble of defining PRIu64 and changing it to %llu or %lu depending on the architecture, like is done in inttypes.h, but upon inspection, it appears that the Linux kernel treats %llu and %lu the same, no matter whether the system is 64-bit or 32-bit, so getting it right is a moot point. The FreeBSD behavior was not checked. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13956 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
ZED: Enforce format specifier correctness While experimenting with ways to reduce false positives in static analyzers, I discovered that we could get the compiler to enforce correctness for format specifiers by annotating function prototypes. This patch only applies to ZED, since we are not annotating things that affect the kernel, such as zfs_panic_recover(). Unfortunately, we do not have inttypes.h inside the kernel, so we need to hard code format specifiers if we annotate things that affect the kernel. However, GCC has this interesting behavior on amd64 where if you use %llu with uint64_t, it will complain that uint64_t is unsigned long long int and suggests %lu, but if you actually use %lu, it will complain that uint64_t is unsigned long int. We could go through the trouble of defining PRIu64 and changing it to %llu or %lu depending on the architecture, like is done in inttypes.h, but upon inspection, it appears that the Linux kernel treats %llu and %lu the same, no matter whether the system is 64-bit or 32-bit, so getting it right is a moot point. The FreeBSD behavior was not checked. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13956 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Fix unreachable code in zstreamdump 82226e4f44baa3f7c3101caaaf941927aa318e46 was intended to prevent a warning from being printed in situations where it was inappropriate, but accidentally disabled it entirely by setting featureflags in the wrong case statement. Coverity reported this as dead code. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13946 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Reduce false positives from Static Analyzers Both Clang's Static Analyzer and Synopsys' Coverity would ignore assertions. Following Clang's advice, we annotate our assertions: https://clang-analyzer.llvm.org/annotations.html#custom_assertions This makes both Clang's Static Analyzer and Coverity properly identify assertions. This change reduced Clang's reported defects from 246 to 180. It also reduced the false positives reported by Coverityi by 10, while enabling Coverity to find 9 more defects that previously were false negatives. A couple examples of this would be CID-1524417 and CID-1524423. After submitting a build to coverity with the modified assertions, CID-1524417 disappeared while the report for CID-1524423 no longer claimed that the assertion tripped. Coincidentally, it turns out that it is possible to more accurately annotate our headers than the Coverity modelling file permits in the case of format strings. Since we can do that and this patch annotates headers whenever __coverity_panic__() would have been used in the model file, we drop all models that use __coverity_panic__() from the model file. Upon seeing the success in eliminating false positives involving assertions, it occurred to me that we could also modify our headers to eliminate coverity's false positives involving byte swaps. We now have coverity specific byteswap macros, that do nothing, to disable Coverity's false positives when we do byte swaps. This allowed us to also drop the byteswap definitions from the model file. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13902 part 1/1 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tony Hutter
hutter2@llnl.gov |
|
|
|
||||||||||||||||||||
|
Tag zfs-2.1.6 META file and changelog updated. Signed-off-by: Tony Hutter <hutter2@llnl.gov> Pull-request: #13886 part 74/74 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables than uint32_t because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables than uint32_t because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint32_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint32_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t and use uint_t for all of the parameters that we made unsigned. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint32_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint32_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t and use uint_t for all of the parameters that we made unsigned. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint32_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint32_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t and use uint_t for all of the parameters that we made unsigned. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
Richard Yao
richard.yao@alumni.stonybrook.edu |
|
|
|
||||||||||||||||||||
|
Cleanup: Specify unsignedness on things that should not be signed In #13871, zfs_vdev_aggregation_limit_non_rotating and zfs_vdev_aggregation_limit being signed was pointed out as a possible reason not to eliminate an unnecessary MAX(unsigned, 0) since the unsigned value was assigned from them. There is no reason for these module parameters to be signed and upon inspection, it was found that there are a number of other module parameters that are signed, but should not be, so we make them unsigned. Making them unsigned made it clear that some other variables in the code should also be unsigned, so we also make those unsigned. This prevents users from setting negative values that could potentially cause bad behaviors. It also makes the code slightly easier to understand. Mostly module parameters that deal with timeouts, limits, bitshifts and percentages are made unsigned by this. Any that are boolean are left signed, since whether booleans should be considered signed or unsigned does not matter. Making zfs_arc_lotsfree_percent unsigned caused a `zfs_arc_lotsfree_percent >= 0` check to become redundant, so it was removed. Removing the check was also necessary to prevent a compiler error from -Werror=type-limits. Several end of line comments had to be moved to their own lines because replacing int with uint32_t caused us to exceed the 80 character limit enforced by cstyle.pl. The following were kept signed because they are passed to taskq_create(), which expects signed values and modifying the OpenSolaris/Illumos DDI is out of scope of this patch: * metaslab_load_pct * zfs_sync_taskq_batch_pct * zfs_zil_clean_taskq_nthr_pct * zfs_zil_clean_taskq_minalloc * zfs_zil_clean_taskq_maxalloc * zfs_arc_prune_task_threads Also, negative values in those parameters was found to be harmless. The following were left signed because either negative values make sense, or more analysis was needed to determine whether negative values should be disallowed: * zfs_metaslab_switch_threshold * zfs_pd_bytes_max * zfs_livelist_min_percent_shared zfs_multihost_history was made static to be consistent with other parameters. A number of module parameters were marked as signed, but in reality referenced unsigned variables. upgrade_errlog_limit is one of the numerous examples. In the case of zfs_vdev_async_read_max_active, it was already uint32_t, but zdb had an extern int declaration for it. Interestingly, the documentation in zfs.4 was right for upgrade_errlog_limit despite the module parameter being wrongly marked, while the documentation for zfs_vdev_async_read_max_active (and friends) was wrong. It was also wrong for zstd_abort_size, which was unsigned, but was documented as signed. Also, the documentation in zfs.4 incorrectly described the following parameters as ulong when they were int: * zfs_arc_meta_adjust_restarts * zfs_override_estimate_recordsize They are now uint32_t as of this patch and thus the man page has been updated to describe them as uint. dbuf_state_index was left alone since it does nothing and perhaps should be removed in another patch. If any module parameters were missed, they were not found by `grep -r 'ZFS_MODULE_PARAM' | grep ', INT'`. I did find a few that grep missed, but only because they were in files that had hits. This patch intentionally did not attempt to address whether some of these module parameters should be elevated to 64-bit parameters, because the length of a long on 32-bit is 32-bit. Lastly, it was pointed out during review that uint_t is a better match for these variables because FreeBSD kernel parameter definitions are designed for uint_t, whose bit width can change in future memory models. As a result, we change the existing parameters that are uint32_t to use uint_t and use uint_t for all of the parameters that we made unsigned. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Pull-request: #13875 part 1/1 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Ameer Hamza
ahamza@ixsystems.com |
|
|
|
||||||||||||||||||||
|
zed: mark disks as REMOVED when they are removed ZED does not take any action for disk removal events if there is no spare VDEV available. Added zpool_vdev_remove_wanted() in libzfs and vdev_remove_wanted() in vdev.c to remove the VDEV through ZED on removal event. This means that if you are running zed and remove a disk, it will be propertly marked as REMOVED. Signed-off-by: Ameer Hamza <ahamza@ixsystems.com> Pull-request: #13797 part 1/1 |
|||||||||||||||||||||||
|
Ameer Hamza
ahamza@ixsystems.com |
|
|
|
||||||||||||||||||||
|
zed: mark disks as REMOVED when they are removed ZED does not take any action for disk removal events if there is no spare VDEV available. Added zpool_vdev_remove_wanted() in libzfs and vdev_remove_wanted() in vdev.c to remove the VDEV through ZED on removal event. This means that if you are running zed and remove a disk, it will be propertly marked as REMOVED. Signed-off-by: Ameer Hamza <ahamza@ixsystems.com> Pull-request: #13797 part 1/1 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Update BLAKE3 for using the new impl handling This commit changes the BLAKE3 implementation handling and also the calls to it from the ztest command. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 4/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add generic implementation handling and new SHA2 impl The skeleton file module/icp/include/generic_impl.c can be used for iterating over different implementations of algorithms. It's used by SHA256, SHA512 and BLAKE3 currently. The generic SHA2 implementation is based on public domain code of ccpcrypto v0.10. The assembly files are taken from current openssl master and are licensed under the Apache License Version 2.0. sha256-x86_64.S: x86-64, SSSE3, AVX, AVX2, SHA-NI sha512-x86_64.S: x86-64,AVX, AVX2 sha256-armv7.S: ... sha512-armv7.S: ... sha256-armv8.S: NEON, ARMv8 Cryptography Extension sha512-armv8.S: ARMv8 Cryptography Extension sha256-ppc.S: generic PPC64 LE/BE sha512-ppc.S: generic PPC64 LE/BE sha256-p8.S: Power ISA Version 2.07 LE/BE sha512-p8.S: Power ISA Version 2.07 LE/BE Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 3/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add SHA_NI instruction check to the SIMD headers This commit adds the function zfs_shani_available() to the libspl, os/freebsd and os/linux headers. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 2/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Remove old or redundant SHA2 files We had three sha2.h headers in different places. The BSD spl version, the linux spl version and the generic solaris version. The only assembly used for acceleration was some old x86-64 openssl implementation for sha256 within the icp module. On FreeBSD the whole SHA2 files of FreeBSD were copied into OpenZFS, these files got removed also. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 1/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Update BLAKE3 for using the new impl handling This commit changes the BLAKE3 implementation handling and also the calls to it from the ztest command. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 4/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add generic implementation handling and new SHA2 impl The skeleton file module/icp/include/generic_impl.c can be used for iterating over different implementations of algorithms. It's used by SHA256, SHA512 and BLAKE3 currently. The generic SHA2 implementation is based on public domain code of ccpcrypto v0.10. The assembly files are taken from current openssl master and are licensed under the Apache License Version 2.0. sha256-x86_64.S: x86-64, SSSE3, AVX, AVX2, SHA-NI sha512-x86_64.S: x86-64,AVX, AVX2 sha256-armv7.S: ... sha512-armv7.S: ... sha256-armv8.S: NEON, ARMv8 Cryptography Extension sha512-armv8.S: ARMv8 Cryptography Extension sha256-ppc.S: generic PPC64 LE/BE sha512-ppc.S: generic PPC64 LE/BE sha256-p8.S: Power ISA Version 2.07 LE/BE sha512-p8.S: Power ISA Version 2.07 LE/BE Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 3/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add SHA_NI instruction check to the SIMD headers This commit adds the function zfs_shani_available() to the libspl, os/freebsd and os/linux headers. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 2/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Remove old or redundant SHA2 files We had three sha2.h headers in different places. The BSD spl version, the linux spl version and the generic solaris version. The only assembly used for acceleration was some old x86-64 openssl implementation for sha256 within the icp module. On FreeBSD the whole SHA2 files of FreeBSD were copied into OpenZFS, these files got removed also. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 1/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Update BLAKE3 for using the new impl handling This commit changes the BLAKE3 implementation handling and also the calls to it from the ztest command. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 4/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add generic implementation handling and new SHA2 impl The skeleton file module/icp/include/generic_impl.c can be used for iterating over different implementations of algorithms. It's used by SHA256, SHA512 and BLAKE3 currently. The generic SHA2 implementation is based on public domain code of ccpcrypto v0.10. The assembly files are taken from current openssl master and are licensed under the Apache License Version 2.0. sha256-x86_64.S: x86-64, SSSE3, AVX, AVX2, SHA-NI sha512-x86_64.S: x86-64,AVX, AVX2 sha256-armv7.S: ... sha512-armv7.S: ... sha256-armv8.S: NEON, ARMv8 Cryptography Extension sha512-armv8.S: ARMv8 Cryptography Extension sha256-ppc.S: generic PPC64 LE/BE sha512-ppc.S: generic PPC64 LE/BE sha256-p8.S: Power ISA Version 2.07 LE/BE sha512-p8.S: Power ISA Version 2.07 LE/BE Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 3/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add SHA_NI instruction check to the SIMD headers This commit adds the function zfs_shani_available() to the libspl, os/freebsd and os/linux headers. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 2/4 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Remove old or redundant SHA2 files We had three sha2.h headers in different places. The BSD spl version, the linux spl version and the generic solaris version. The only assembly used for acceleration was some old x86-64 openssl implementation for sha256 within the icp module. On FreeBSD the whole SHA2 files of FreeBSD were copied into OpenZFS, these files got removed also. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 1/4 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
geändert: algs/sha2/sha256_impl.c geändert: algs/sha2/sha512_impl.c geändert: asm-aarch64/sha2/sha256-armv8.S geändert: asm-aarch64/sha2/sha512-armv8.S geändert: asm-arm/sha2/sha256-armv7.S geändert: asm-arm/sha2/sha512-armv7.S Pull-request: #13741 part 5/5 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Update BLAKE3 for using the new impl handling This commit changes the BLAKE3 implementation handling and also the calls to it from the ztest command. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 4/5 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add generic implementation handling and new SHA2 impl The skeleton file module/icp/include/generic_impl.c can be used for iterating over different implementations of algorithms. It's used by SHA256, SHA512 and BLAKE3 currently. The generic SHA2 implementation is based on public domain code of ccpcrypto v0.10. The assembly files are taken from current openssl master and are licensed under the Apache License Version 2.0. sha256-x86_64.S: x86-64, SSSE3, AVX, AVX2, SHA-NI sha512-x86_64.S: x86-64,AVX, AVX2 sha256-armv7.S: ... sha512-armv7.S: ... sha256-armv8.S: NEON, ARMv8 Cryptography Extension sha512-armv8.S: ARMv8 Cryptography Extension sha256-ppc.S: generic PPC64 LE/BE sha512-ppc.S: generic PPC64 LE/BE sha256-p8.S: Power ISA Version 2.07 LE/BE sha512-p8.S: Power ISA Version 2.07 LE/BE Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 3/5 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Add SHA_NI instruction check to the SIMD headers This commit adds the function zfs_shani_available() to the libspl, os/freebsd and os/linux headers. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 2/5 |
|||||||||||||||||||||||
|
Tino Reichardt
milky-zfs@mcmilk.de |
|
|
|
||||||||||||||||||||
|
Remove old or redundant SHA2 files We had three sha2.h headers in different places. The BSD spl version, the linux spl version and the generic solaris version. The only assembly used for acceleration was some old x86-64 openssl implementation for sha256 within the icp module. On FreeBSD the whole SHA2 files of FreeBSD were copied into OpenZFS, these files got removed also. Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Pull-request: #13741 part 1/5 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Jason Lee
jasonlee@lanl.gov |
|
|
|
||||||||||||||||||||
|
ZFS Interface for Accelerators (Z.I.A.) The ZIO write pipeline has been modified to allow for external, alternative implementations of operations to be used. The original ZFS functions remain in the code as fallback in case the external implementation fails. Definitions: Accelerator - an entity (usually hardware) that is intended to accelerate operations Offloader - synonym of accelerator; used interchangeably Data Processing Unit Services Module (DPUSM) - https://github.com/hpc/dpusm - defines a "provider API" for accelerator vendors to set up - defines a "user API" for accelerator consumers to call - maintains list of providers and coordinates interactions between providers and consumers. Provider - a DPUSM wrapper for an accelerator's API Offload - moving data from ZFS/memory to the accelerator Onload - the opposite of offload In order for Z.I.A. to be extensible, it does not directly communicate with a fixed accelerator. Rather, Z.I.A. acquires a handle to a DPUSM, which is then used to acquire handles to providers. Using ZFS with Z.I.A.: 1. Build and start the DPUSM 2. Implement, build, and register a provider with the DPUSM 3. Reconfigure ZFS with '--with-zia=<DPUSM root>' 4. Rebuild and start ZFS 5. Create a zpool 6. Select the provider zpool set zia_provider=<provider name> <zpool> 7. Select operations to offload zpool set zia_<property>=on <zpool> The functions that can be replaced with alternative operations are: - compression - data is offloaded and then compressed - metadata is compressed in-memory and then offloaded - decompression can be replaced, but the replacement function is not called anywhere - checksum - checksum compute and checksum error call the same function - raidz - generation - reconstruction - vdev_file - open - write - close - vdev_disk - open - invalidate - write - flush - close abd_t, raidz_row_t, and vdev_t have each been given an additional "void *<prefix>_zia_handle" member. These opaque handles point to data that is located on an offloader. abds are still allocated, but their contents are expected to diverge from the offloaded copy as operations are run. The modifications to ZFS can be thought of as two sets of changes: - The ZIO write pipeline - compression, checksum, RAIDZ generation, and write - Each stage starts by offloading data that was not previously offloaded - This allows for ZIOs to be offloaded at any point in these stages - Successful operations do not onload back into memory between stages - Errors cause data to be onloaded, or dropped if the copy in memory matches the offloaded copy - This might cause thrashing, but should not happen often, as the intention is for all of the stages to be offloaded, and thus not require onloading - Resilver - RAIDZ reconstruction, checksum, RAIDZ generation, and write - Because resilver is only one stage in the ZIO pipeline, data is only offloaded once at the beginning - Errors cause data to be onloaded, but will not re-offload in subsequent steps within resilver ARC compression is disabled when Z.I.A. is enabled Aggregation is disabled for offloaded abds RPMs will build with Z.I.A. Added example provider in module/zia-software-provider Signed-off-by: Jason Lee <jasonlee@lanl.gov> Pull-request: #13628 part 1/1 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Jorgen Lundman
lundman@lundman.net |
|
|
|
||||||||||||||||||||
|
macOS: cstyle fixes Pull-request: #12110 part 89/89 |
|||||||||||||||||||||||
|
Jorgen Lundman
lundman@lundman.net |
|
|
|
||||||||||||||||||||
|
macOS: replace bcmp / bcopy / bfree Pull-request: #12110 part 88/88 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||
|
Jorgen Lundman
lundman@lundman.net |
|
|
|
||||||||||||||||||||
|
macOS: fixes and updates Pull-request: #12110 part 87/87 |
|||||||||||||||||||||||
|
|||||||||||||||||||||||