fix zfs send progress reporting #10216
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ahrens
added
Status: Code Review Needed
|
The existing code works for us, so yes some platform-dependent macro or inline function will be needed. |
ahrens
force-pushed
the
send_progress
branch
2 times, most recently
from
a3ea992
to
816e606
Compare
The progress of a send is supposed to be reported by `zfs send -v`, but it is not. This works by creating a new user thread (with pthread_create()) which does ZFS_IOC_SEND_PROGRESS ioctls to check how much progress has been made. This IOCTL finds the specified send (since there may be multiple concurrent sends in the system). The IOCTL also checks that the specified send was started by the current process. On Linux, different threads of the same process are represented as different `struct task_struct`s (and, confusingly, have different PID's). To check if if two threads are in the same process, we need to check if they have the same `struct task_struct:group_leader`. We used to to this correctly, but it was inadvertently changed by 30af21b (Redacted Send) to simply check if the current `struct task_struct` is the one that started the send. This commit changes the code back to checking if the send was started by a `struct task_struct` with the same `group_leader` as the calling thread. Signed-off-by: Matthew Ahrens <mahrens@delphix.com> Closes openzfs#10215
behlendorf
approved these changes
Apr 17, 2020
pcd1193182
approved these changes
Apr 17, 2020
behlendorf
added
Status: Accepted
12 tasks
jsai20
pushed a commit
to jsai20/zfs
that referenced
this pull request
Mar 30, 2021
The progress of a send is supposed to be reported by `zfs send -v`, but it is not. This works by creating a new user thread (with pthread_create()) which does ZFS_IOC_SEND_PROGRESS ioctls to check how much progress has been made. This IOCTL finds the specified send (since there may be multiple concurrent sends in the system). The IOCTL also checks that the specified send was started by the current process. On Linux, different threads of the same process are represented as different `struct task_struct`s (and, confusingly, have different PID's). To check if if two threads are in the same process, we need to check if they have the same `struct task_struct:group_leader`. We used to to this correctly, but it was inadvertently changed by 30af21b (Redacted Send) to simply check if the current `struct task_struct` is the one that started the send. This commit changes the code back to checking if the send was started by a `struct task_struct` with the same `group_leader` as the calling thread. Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Reviewed-by: Chris Wedgwood <cw@f00f.org> Reviewed-by: Paul Dagnelie <pcd@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Matthew Ahrens <mahrens@delphix.com> Closes openzfs#10215 Closes openzfs#10216
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
The progress of a send is supposed to be reported by
zfs send -v, butit is not. This works by creating a new user thread (with
pthread_create()) which does ZFS_IOC_SEND_PROGRESS ioctls to check how
much progress has been made. This IOCTL finds the specified send (since
there may be multiple concurrent sends in the system). The IOCTL also
checks that the specified send was started by the current process.
On Linux, different threads of the same process are represented as
different
struct task_structs (and, confusingly, have differentPID's). To check if if two threads are in the same process, we need to
check if they have the same
struct task_struct:group_leader.We used to to this correctly, but it was inadvertently changed by
30af21b (Redacted Send) to simply check if the current
struct task_structis the one that started the send.Closes #10215
Description
This commit changes the code back to checking if the send was started by
a
struct task_structwith the samegroup_leaderas the callingthread.
Note: this probably doesn't work on FreeBSD and I'll need to figure out the right way to abstract this out. Therefore marking this PR as Draft.
Note: Redacted Send is not in 0.8, so this bug only exists on master (AFAIK).
How Has This Been Tested?
Manual testing.
Types of changes
Checklist:
Signed-off-by.