Skip to content

Add SIGSTOP and SIGTSTP handling to issig #11801

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.

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

Merged
merged 3 commits into from
Apr 15, 2021
Merged

Conversation

pcd1193182
Copy link
Contributor

@pcd1193182 pcd1193182 commented Mar 25, 2021

Motivation and Context

Currently, ZFS send and receive die if you send them SIGSTOP or SIGTSTP. This was originally raised in #810, and was supposedly fixed in #10843, but that fix doesn't appear to contain any relevant logic to this issue.

Description

This change adds SIGSTOP and SIGTSTP handling to the issig function; this mirrors its behavior on Solaris. This way, long running kernel tasks can be stopped with the appropriate signals. Note that doing so with ctrl-z on the command line doesn't return control of the tty to the shell, because tty handling is done separately from stopping the process. That can be future work, if people feel that it is a necessary addition.

Note that this problem also exists on FreeBSD (I believe; haven't tested manually) and this PR does not address that.

How Has This Been Tested?

ZFS test suite passed, changes loaded onto a VM and SIGSTOP sent to running sends/receives. Verified that the processes stopped when the signals were received, and then resumed when sent SIGCONT.

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:

@pcd1193182 pcd1193182 requested review from behlendorf and ahrens March 25, 2021 22:38
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Mar 26, 2021
Copy link
Member

@ahrens ahrens left a comment

Choose a reason for hiding this comment

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

The new behavior sounds good to me.

@pcd1193182 pcd1193182 force-pushed the signals branch 2 times, most recently from c1b5772 to 63f2095 Compare April 12, 2021 20:10
@pcd1193182 pcd1193182 force-pushed the signals branch 2 times, most recently from 9d03d9a to 919f2fa Compare April 13, 2021 18:02
Signed-off-by: Paul Dagnelie <[email protected]>
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 14, 2021
Signed-off-by: Paul Dagnelie <[email protected]>
@behlendorf behlendorf merged commit 414f724 into openzfs:master Apr 15, 2021
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Apr 21, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function; 
this mirrors its behavior on Solaris. This way, long running kernel 
tasks can be stopped with the appropriate signals. Note that doing 
so with ctrl-z on the command line doesn't return control of the tty 
to the shell, because tty handling is done separately from stopping 
the process. That can be future work, if people feel that it is a 
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue openzfs#810 
Issue openzfs#10843 
Closes openzfs#11801
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function; 
this mirrors its behavior on Solaris. This way, long running kernel 
tasks can be stopped with the appropriate signals. Note that doing 
so with ctrl-z on the command line doesn't return control of the tty 
to the shell, because tty handling is done separately from stopping 
the process. That can be future work, if people feel that it is a 
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue openzfs#810 
Issue openzfs#10843 
Closes openzfs#11801
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 7, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function;
this mirrors its behavior on Solaris. This way, long running kernel
tasks can be stopped with the appropriate signals. Note that doing
so with ctrl-z on the command line doesn't return control of the tty
to the shell, because tty handling is done separately from stopping
the process. That can be future work, if people feel that it is a
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue openzfs#810
Issue openzfs#10843
Closes openzfs#11801
tonyhutter pushed a commit that referenced this pull request Sep 21, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function;
this mirrors its behavior on Solaris. This way, long running kernel
tasks can be stopped with the appropriate signals. Note that doing
so with ctrl-z on the command line doesn't return control of the tty
to the shell, because tty handling is done separately from stopping
the process. That can be future work, if people feel that it is a
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue #810
Issue #10843
Closes #11801
tonyhutter pushed a commit that referenced this pull request Sep 22, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function;
this mirrors its behavior on Solaris. This way, long running kernel
tasks can be stopped with the appropriate signals. Note that doing
so with ctrl-z on the command line doesn't return control of the tty
to the shell, because tty handling is done separately from stopping
the process. That can be future work, if people feel that it is a
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue #810
Issue #10843
Closes #11801
ghost pushed a commit to truenas/zfs that referenced this pull request Oct 7, 2021
This change adds SIGSTOP and SIGTSTP handling to the issig function;
this mirrors its behavior on Solaris. This way, long running kernel
tasks can be stopped with the appropriate signals. Note that doing
so with ctrl-z on the command line doesn't return control of the tty
to the shell, because tty handling is done separately from stopping
the process. That can be future work, if people feel that it is a
necessary addition.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Matthew Ahrens <[email protected]>
Signed-off-by: Paul Dagnelie <[email protected]>
Issue openzfs#810
Issue openzfs#10843
Closes openzfs#11801
@pcd1193182 pcd1193182 deleted the signals branch November 5, 2021 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants