Skip to content
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

Make userspace libs more threadsafe #15793

Closed
wants to merge 2 commits into from
Closed

Make userspace libs more threadsafe #15793

wants to merge 2 commits into from

Conversation

rkojedzinszky
Copy link
Contributor

@rkojedzinszky rkojedzinszky commented Jan 19, 2024 •
edited
Loading

Motivation and Context

Improves the userspace lib to be threadsafe.

Description

Replaced non-threadsafe functions with their threadsafe counterparts in lib/.

How Has This Been Tested?

Compiled the code on linux/freebsd.

Compiled on Debian bookworm, FreeBSD 13.2.

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:

@rkojedzinszky rkojedzinszky mentioned this pull request Jan 19, 2024
Open
@rkojedzinszky rkojedzinszky force-pushed the threadsafe-lib branch 2 times, most recently from 706ac95 to 085f500 Compare January 19, 2024 21:56
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 19, 2024
@rkojedzinszky
libzfs: make userquota_propname_decode threadsafe
b41d640 
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
@rkojedzinszky
libzfs: use zfs_strerror() in place of strerror()
198adde 
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 26, 2024
@rkojedzinszky
Copy link
Contributor Author

Thanks for the reviews! Do anyone has an idea on how to prevent future modifications to lib/ which would break thread safety? How to detect calls to libc functions that are not threadsafe, or how to detect non thread-local global variable usage?

@behlendorf
Copy link
Contributor

One idea would be to add a codeql checker for any libc functions which are not threadsafe. This way any PRs attempting to use these functions will be caught by the CI. We already do something similar for deprecated functions we want to keep out of the code base. That of course won't handle everything, but it's a start.

https://github.com/openzfs/zfs/blob/master/.github/codeql/custom-queries/cpp/deprecatedFunctionUsage.ql

behlendorf pushed a commit that referenced this pull request Jan 29, 2024
@rkojedzinszky @behlendorf
libzfs: use zfs_strerror() in place of strerror()
401c356 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes #15793
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@rkojedzinszky @lundman
libzfs: make userquota_propname_decode threadsafe
b59b39c 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes openzfs#15793
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@rkojedzinszky @lundman
libzfs: use zfs_strerror() in place of strerror()
74c846c 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes openzfs#15793
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@rkojedzinszky @lundman
libzfs: make userquota_propname_decode threadsafe
153264b 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes openzfs#15793
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@rkojedzinszky @lundman
libzfs: use zfs_strerror() in place of strerror()
94fc8f3 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes openzfs#15793
@yocalebo yocalebo mentioned this pull request Sep 6, 2024
Closed
13 tasks
ptr1337 pushed a commit to CachyOS/zfs that referenced this pull request Nov 14, 2024
@rkojedzinszky @tonyhutter
libzfs: use zfs_strerror() in place of strerror()
a1ea40a 
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de>
Signed-off-by: Richard Kojedzinszky <richard@kojedz.in>
Closes openzfs#15793
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcmilk mcmilk mcmilk approved these changes

@behlendorf behlendorf behlendorf 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
@rkojedzinszky @behlendorf @mcmilk