[PATCH v1] drmgr: remove sig_setup(), sighandler()

[Scott Cheloha]

Nothing in sighandler() is async-safe. Most of the signals it handles
are fatal and indicate a programmer error.

Removing sig_setup() and sighandler() makes drmgr more async-safe.

Signed-off-by: Scott Cheloha <che...@linux.ibm.com>
---
src/drmgr/common.c | 102 ---------------------------------------------
1 file changed, 102 deletions(-)

diff --git a/src/drmgr/common.c b/src/drmgr/common.c
index 9cd91d1..70a7929 100644
--- a/src/drmgr/common.c
+++ b/src/drmgr/common.c
@@ -214,15 +214,6 @@ inline int dr_init(void)
say(DEBUG, "\n########## %s ##########\n", tbuf);
}

- /* Mask signals so we do not get interrupted */
- if (sig_setup()) {
- say(ERROR, "Could not mask signals to avoid interrupts\n");
- if (log_fd)
- close(log_fd);
- dr_unlock();
- return -1;
- }
-
rc = check_kmods();
if (rc) {
if (log_fd)
@@ -869,99 +860,6 @@ get_property_size(const char *path, const char *property)
return sb.st_size;
}

-/**
- * sighandler
- * @brief Simple signal handler to print signal/stack info, cleanup and exit.
- *
- * @param signo signal number we caught.
- */
-void
-sighandler(int signo)
-{
- say(ERROR, "Received signal %d, attempting to cleanup and exit\n",
- signo);
-
-#ifdef __GLIBC__
- if (log_fd) {
- void *callstack[128];
- int sz;
-
- sz = backtrace(callstack, 128);
- backtrace_symbols_fd(callstack, sz, log_fd);
- }
-#endif
-
- dr_fini();
- exit(-1);
-}
-
-/**
- * sig_setup
- * @brief Mask signals so that dynamic reconfig operations won't be
- * interrupted and catch others.
- *
- * @returns 0 on success, !0 otherwise
- */
-int
-sig_setup(void)
-{
- sigset_t sigset;
- struct sigaction sigact;
- void *callstack[128];
- int rc;
-
- /* Now set up a mask with all signals masked */
- sigfillset(&sigset);
-
- /* Clear mask bits for signals we don't want to mask */
- sigdelset(&sigset, SIGBUS);
- sigdelset(&sigset, SIGXFSZ);
- sigdelset(&sigset, SIGSEGV);
- sigdelset(&sigset, SIGTRAP);
- sigdelset(&sigset, SIGILL);
- sigdelset(&sigset, SIGFPE);
- sigdelset(&sigset, SIGSYS);
- sigdelset(&sigset, SIGPIPE);
- sigdelset(&sigset, SIGVTALRM);
- sigdelset(&sigset, SIGALRM);
- sigdelset(&sigset, SIGQUIT);
- sigdelset(&sigset, SIGABRT);
-
- /* Now block all remaining signals */
- rc = sigprocmask(SIG_BLOCK, &sigset, NULL);
- if (rc)
- return -1;
-
- /* Now set up a signal handler for the signals we want to catch */
- memset(&sigact, 0, sizeof(sigact));
- sigemptyset(&sigact.sa_mask);
- sigact.sa_handler = sighandler;
-
- if (sigaction(SIGQUIT, &sigact, NULL))
- return -1;
-
- if (sigaction(SIGILL, &sigact, NULL))
- return -1;
-
- if (sigaction(SIGABRT, &sigact, NULL))
- return -1;
-
- if (sigaction(SIGFPE, &sigact, NULL))
- return -1;
-
- if (sigaction(SIGSEGV, &sigact, NULL))
- return -1;
-
- if (sigaction(SIGBUS, &sigact, NULL))
- return -1;
-
-#ifdef __GLIBC__
- /* dummy call to backtrace to get symbol loaded */
- backtrace(callstack, 128);
-#endif
- return 0;
-}
-
char *php_slot_type_msg[]={
"",
"PCI 32 bit, 33MHz, 5 volt slot",
--
2.31.1

[Nathan Lynch]

Scott Cheloha <che...@linux.ibm.com> writes:
> Nothing in sighandler() is async-safe.

The precise term to use is async-signal-safe.

https://www.gnu.org/software/libc/manual/html_node/POSIX-Safety-Concepts.html

> Most of the signals it handles
> are fatal and indicate a programmer error.
>
> Removing sig_setup() and sighandler() makes drmgr more async-safe.
>
> Signed-off-by: Scott Cheloha <che...@linux.ibm.com>

Will you add "Fixes #56" in the commit message in v2, so it will get
closed once the change hits the master branch?

https://github.com/ibm-power-utilities/powerpc-utils/issues/56

> ---
> src/drmgr/common.c | 102 ---------------------------------------------
> 1 file changed, 102 deletions(-)

$ git grep -n -e sig_setup
src/drmgr/common.c:218: if (sig_setup()) {
src/drmgr/common.c:899: * sig_setup
src/drmgr/common.c:906:sig_setup(void)
src/drmgr/dr.h:119:int sig_setup(void);
src/drmgr/lsslot_chrp_cpu.c:135: if (sig_setup()) {

To be complete (and not break the build), this needs to remove the
declaration from dr.h and the other caller in lsslot_chrp_cpu.c.