Clean up source to get Linux builds working #119

andrewc12 · 2022-08-06T12:24:08Z

Motivation and Context

In my quest to improve the testing situation for windows I accidently stumbled across the idea of cleaning up the source to be able to get both windows and linux builds working.
This should let us use both the automated linux builds to test for large breaking changes and serve as the groundwork for either using the linux tests under wsl or to port the tests to natively to windows.

Description

moving windows specific code into windows specific source files
enclosing windows specific code in conditionals
creating defines for new functions in other os source files
changing some types to macros that support multiple os

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)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
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.

from zfs_ioctl.h to zfs_ioctl_os.c

andrewc12 · 2022-08-06T12:30:55Z

@datacore-rm you might want to give this a look to see if it causes problems with perfmon counters.

datacore-rm · 2022-08-08T07:08:15Z

Changes looks fine.Thanks.

andrewc12 · 2022-08-10T01:55:11Z

Added a workflow to actually build the code on linux

lundman · 2022-08-10T02:46:24Z

module/zfs/zvol.c

@@ -1740,6 +1740,9 @@ zvol_fini_impl(void)
 }

 /* ZFS ZVOLDI */
+// HACK FIXME


Shouldn't these be in zvol_os.c ?

lundman · 2022-08-10T02:46:54Z

module/zfs/dsl_pool.c

@@ -117,7 +117,7 @@ ZFS_MODULE_ULONG zfs_wrlog_data_max = 0;
 * zfs_dirty_data_max), push out a txg.  This should be less than
 * zfs_vdev_async_write_active_min_dirty_percent.
 */


we are removing static?

lundman · 2022-08-10T02:47:57Z

include/sys/vdev_file.h

+#else
+/* Grabbed from https://github.com/openzfs/zfs/blob/1d3ba0bf01020f5459b1c28db3979129088924c0/include/sys/vdev_file.h#L36 */
+typedef struct vdev_file {
+    zfs_file_t* vf_file;


nit: "*vf_file;"

lundman · 2022-08-10T02:48:29Z

cmd/zpool/os/linux/zpool_vdev_os.c

@@ -417,9 +417,3 @@ check_file(const char *file, boolean_t force, boolean_t isspare)
 {
 	return (check_file_generic(file, force, isspare));
 }
-


there were two? hah neat

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

in reference to openzfsonwindows#119 (comment)

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

in reference to openzfsonwindows#119 (comment)

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

in reference to openzfsonwindows#119 (comment)

* Move perfmon functions to windows from zfs_ioctl.h to zfs_ioctl_os.c * remove static for zfs_dirty_data_sync_percent * uint64_t to ZFS_MODULE_ULONG * add ifdef _WIN32 to zvol.c * define posix_memalign_free for other os * add vdev_file_t for other os * remove duplicate check_file * Create build_for_wsl.yaml * move functions to zvol_os.c and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment) * Fix code formatting in reference to openzfsonwindows#119 (comment) * revert the removal of static in dsl_pool.c * remove zfs_dirty_data_sync_percent in dsl_pool.h * make zvol_find_by_name not static * changed workflow name * cstyle zpool_vdev_os.c

Unortunately, Windows defines 'long' as 32-bit even on x64 compiles. We create two new macros ZFS_MODULE_LONG and ZFS_MODULE_ULONG. These two will be 'long' on Unix, and let the toolchain handle the size of it. On Windows the two macros are defined as 'int64_t'/'uint64_t'. Clean up source to get Linux builds working (openzfs#119) * Move perfmon functions to windows from zfs_ioctl.h to zfs_ioctl_os.c * remove static for zfs_dirty_data_sync_percent * uint64_t to ZFS_MODULE_ULONG * add ifdef _WIN32 to zvol.c * define posix_memalign_free for other os * add vdev_file_t for other os * remove duplicate check_file * Create build_for_wsl.yaml * move functions to zvol_os.c and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment) * Fix code formatting in reference to openzfsonwindows#119 (comment) * revert the removal of static in dsl_pool.c * remove zfs_dirty_data_sync_percent in dsl_pool.h * make zvol_find_by_name not static * changed workflow name * cstyle zpool_vdev_os.c Fix cstyle complaints (openzfs#125) * clean cstyle * squash * squash * Update zfs_vnops_windows.c * Update zfs_windows_zvol_wmi.c * squash * Additional cstyle fixes * Update sysctl_os.c Upstream: Additional MODULE ULONG Bring per_txg_dirty_frees_percent back to 30 The current value causes significant artificial slowdown during mass parallel file removal, which can be observed both on FreeBSD and Linux when running real workloads. Sample results from Linux doing make -j 96 clean after an allyesconfig modules build: before: 4.14s user 6.79s system 48% cpu 22.631 total after: 4.17s user 6.44s system 153% cpu 6.927 total FreeBSD results in the ticket. Reviewed-by: Alexander Motin <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Reviewed-by: Richard Yao <[email protected]> Reviewed-by: George Melikov <[email protected]> Signed-off-by: Mateusz Guzik <[email protected]> Closes openzfs#13932 Closes openzfs#13938 Co-Authored-By: Jorgen Lundman <[email protected]>

* Move perfmon functions to windows from zfs_ioctl.h to zfs_ioctl_os.c * remove static for zfs_dirty_data_sync_percent * uint64_t to ZFS_MODULE_ULONG * add ifdef _WIN32 to zvol.c * define posix_memalign_free for other os * add vdev_file_t for other os * remove duplicate check_file * Create build_for_wsl.yaml * move functions to zvol_os.c and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment) * Fix code formatting in reference to openzfsonwindows#119 (comment) * revert the removal of static in dsl_pool.c * remove zfs_dirty_data_sync_percent in dsl_pool.h * make zvol_find_by_name not static * changed workflow name * cstyle zpool_vdev_os.c

andrewc12 added 7 commits August 6, 2022 18:45

Move perfmon functions to windows

b845691

from zfs_ioctl.h to zfs_ioctl_os.c

remove static for zfs_dirty_data_sync_percent

491a1e8

uint64_t to ZFS_MODULE_ULONG

0307a40

add ifdef _WIN32 to zvol.c

4760e96

define posix_memalign_free for other os

b97fddb

add vdev_file_t for other os

ae6c750

remove duplicate check_file

e75decc

Create build_for_wsl.yaml

dd5be3b

lundman reviewed Aug 10, 2022

View reviewed changes

andrewc12 added a commit to andrewc12/openzfs that referenced this pull request Aug 10, 2022

move functions to zvol_os.c

870ba9a

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

andrewc12 added a commit to andrewc12/openzfs that referenced this pull request Aug 10, 2022

Fix code formatting

a1b49df

in reference to openzfsonwindows#119 (comment)

andrewc12 added 5 commits August 10, 2022 15:10

move functions to zvol_os.c

41a1df4

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

Fix code formatting

a7d201a

in reference to openzfsonwindows#119 (comment)

revert the removal of static in dsl_pool.c

9855b2c

remove zfs_dirty_data_sync_percent in dsl_pool.h

c8ffb2c

make zvol_find_by_name not static

e280a19

andrewc12 added a commit to andrewc12/openzfs that referenced this pull request Aug 12, 2022

move functions to zvol_os.c

2307113

and add them to the header zvol_impl.h in answer to this discussion openzfsonwindows#119 (comment)

andrewc12 added a commit to andrewc12/openzfs that referenced this pull request Aug 12, 2022

Fix code formatting

906a603

in reference to openzfsonwindows#119 (comment)

changed workflow name

e229024

andrewc12 marked this pull request as draft August 13, 2022 07:37

cstyle zpool_vdev_os.c

9b06ed2

andrewc12 marked this pull request as ready for review August 13, 2022 08:12

lundman merged commit fe81535 into openzfsonwindows:windows Aug 17, 2022

andrewc12 deleted the andrew_linux_build-7 branch August 17, 2022 05:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up source to get Linux builds working #119

Clean up source to get Linux builds working #119

andrewc12 commented Aug 6, 2022

andrewc12 commented Aug 6, 2022

datacore-rm commented Aug 8, 2022

andrewc12 commented Aug 10, 2022

lundman Aug 10, 2022

lundman Aug 10, 2022

lundman Aug 10, 2022

lundman Aug 10, 2022

Clean up source to get Linux builds working #119

Clean up source to get Linux builds working #119

Conversation

andrewc12 commented Aug 6, 2022

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

andrewc12 commented Aug 6, 2022

datacore-rm commented Aug 8, 2022

andrewc12 commented Aug 10, 2022

lundman Aug 10, 2022

Choose a reason for hiding this comment

lundman Aug 10, 2022

Choose a reason for hiding this comment

lundman Aug 10, 2022

Choose a reason for hiding this comment

lundman Aug 10, 2022

Choose a reason for hiding this comment