Skip to content

Extending FreeBSD UIO Struct #11438

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 21, 2021
Merged

Extending FreeBSD UIO Struct #11438

merged 3 commits into from
Jan 21, 2021

Conversation

bwatkinson
Copy link
Contributor

@bwatkinson bwatkinson commented Jan 7, 2021 •
edited
Loading

In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Signed-off-by: Brian Atkinson [email protected]

This patch is to allow for the FreeBSD to extend the uio_t struct. Originally, uio_t was just a typedef to a
struct uio. By embedding the struct uio inside of a uio_t struct further member variables can be added
for FreeBSD.

Motivation and Context

One such case where there is a advantage to this is PR #10018. In order to move the mappings of user pages
in to kernel space into the VFS layers, FreeBSD's uio_t struct can be extended to contain a vm_page_t * and
uio_extflg variables.

Description

I removed the typedef of stuct uio to uio_t and instead placed a struct uio pointer inside of a
uio_t struct. I also added/updated some macros for accessing the member variables of the
underlying struct uio in the uio_t. This allows for common code to still use uio_t's for both
FreeBSD and Linux as was already the case.

How Has This Been Tested?

Ran zloop for 30 mins both Linux and FreeBSD.

Linux: 4.18.0-193
FreeBSD: 12.1 STABLE

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:

lundman
lundman reviewed Jan 7, 2021
View reviewed changes
@bwatkinson bwatkinson force-pushed the unified_uio branch 2 times, most recently from 7e6a08c to 31a8622 Compare January 7, 2021 20:16
behlendorf
behlendorf reviewed Jan 7, 2021
View reviewed changes
@behlendorf behlendorf added the Status: Design Review Needed Architecture or design is under discussion label Jan 7, 2021
@behlendorf behlendorf requested a review from a user January 8, 2021 00:13
behlendorf
behlendorf reviewed Jan 14, 2021
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

This turned out pretty nicely. Just a few small comments.

@behlendorf
Copy link
Contributor

@lundman @freqlabs would you mind having another look at this.

ghost
ghost reviewed Jan 14, 2021
View reviewed changes
lundman
lundman approved these changes Jan 15, 2021
View reviewed changes
@lundman
Copy link
Contributor

lundman commented Jan 15, 2021 •
edited
Loading

The macOS companion commit to this would look something like openzfsonosx/openzfs@4d4f088

quite similar to FreeBSD work, and it is nice to clean up the uio_t header hack. I did not 'do' zio_crypt.c

Also occurs to me that common code now uses znode and not vnode so I will be cleaning up that header hack too, quite pleasing.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Design Review Needed Architecture or design is under discussion labels Jan 20, 2021
@behlendorf
Copy link
Contributor

Looks good! Thanks for running down that last issue, everything looks like it's in good order now.

ghost
ghost approved these changes Jan 20, 2021
View reviewed changes
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.

I also see a few comments in module/zfs/dmu.c referring to uio->uio_loffset but otherwise all the common code looks to be converted based on a quick grep.

@bwatkinson
Extending FreeBSD UIO Struct
2cc79eb 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Signed-off-by: Brian Atkinson <[email protected]>
@bwatkinson
Adding zfs_uio_setoffset()
ac3cf13 
Adding zfs_uio_setoffset() to allow for macOS, FreeBSD, and Linux to
have a common zfs_uio_t interface. This also is a cleaner way of
setting the zfs_uio_t offset as opposed to doing an assignment
through macros.

Signed-off-by: Brian Atkinson <[email protected]>
@bwatkinson
Updates from PR Feedback
96fb570 
Fixing a lingering uio->uio_loffset in the common zfs_vnops code and
updating comments in dmu.c that referred to uio->uio_loffset to now say
zfs_uio_offset(uio).

Signed-off-by: Brian Atkinson <[email protected]>
@behlendorf behlendorf merged commit d0cd9a5 into openzfs:master Jan 21, 2021
@bwatkinson bwatkinson deleted the unified_uio branch January 22, 2021 02:49
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
@bwatkinson @jsai20
Extending FreeBSD UIO Struct
650e989 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
bwatkinson added a commit to bwatkinson/zfs that referenced this pull request May 10, 2021
@bwatkinson
Extending UIO Struct
463b2e3 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
bwatkinson added a commit to bwatkinson/zfs that referenced this pull request May 11, 2021
@bwatkinson
Extending UIO Struct
c869044 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
@bwatkinson
Extending FreeBSD UIO Struct
79670c6 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 6, 2021
@bwatkinson @tonyhutter
Extending FreeBSD UIO Struct
43fa265 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 13, 2021
@bwatkinson @tonyhutter
Extending FreeBSD UIO Struct
0ca646b 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Nov 13, 2021
@bwatkinson @tonyhutter
Extending FreeBSD UIO Struct
0b1e6fc 
In FreeBSD the struct uio was just a typedef to uio_t. In order to
extend this struct, outside of the definition for the struct uio, the
struct uio has been embedded inside of a uio_t struct.

Also renamed all the uio_* interfaces to be zfs_uio_* to make it clear
this is a ZFS interface.

Reviewed-by: Ryan Moeller <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Brian Atkinson <[email protected]>
Closes openzfs#11438
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@lundman lundman lundman approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bwatkinson @behlendorf @lundman