signals: kill block_all_signals() and unblock_all_signals()

It is hardly possible to enumerate all problems with block_all_signals()
and unblock_all_signals().  Just for example,

1. block_all_signals(SIGSTOP/etc) simply can't help if the caller is
   multithreaded. Another thread can dequeue the signal and force the
   group stop.

2. Even is the caller is single-threaded, it will "stop" anyway. It
   will not sleep, but it will spin in kernel space until SIGCONT or
   SIGKILL.

And a lot more. In short, this interface doesn't work at all, at least
the last 10+ years.

Daniel said:

  Yeah the only times I played around with the DRM_LOCK stuff was when
  old drivers accidentally deadlocked - my impression is that the entire
  DRM_LOCK thing was never really tested properly ;-) Hence I'm all for
  purging where this leaks out of the drm subsystem.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Dave Airlie <airlied@redhat.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
index 4924d38..daa2ff1 100644 (file)
--- a/drivers/gpu/drm/drm_lock.c
+++ b/drivers/gpu/drm/drm_lock.c
@@ -38,8 +38,6 @@
 #include "drm_legacy.h"
 #include "drm_internal.h"
 
-static int drm_notifier(void *priv);
-
 static int drm_lock_take(struct drm_lock_data *lock_data, unsigned int context);
 
 /**
@@ -118,14 +116,8 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
         * really probably not the correct answer but lets us debug xkb
         * xserver for now */
        if (!file_priv->is_master) {
-               sigemptyset(&dev->sigmask);
-               sigaddset(&dev->sigmask, SIGSTOP);
-               sigaddset(&dev->sigmask, SIGTSTP);
-               sigaddset(&dev->sigmask, SIGTTIN);
-               sigaddset(&dev->sigmask, SIGTTOU);
                dev->sigdata.context = lock->context;
                dev->sigdata.lock = master->lock.hw_lock;
-               block_all_signals(drm_notifier, dev, &dev->sigmask);
        }
 
        if (dev->driver->dma_quiescent && (lock->flags & _DRM_LOCK_QUIESCENT))
@@ -169,7 +161,6 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_
                /* FIXME: Should really bail out here. */
        }
 
-       unblock_all_signals();
        return 0;
 }
 
@@ -287,38 +278,6 @@ int drm_legacy_lock_free(struct drm_lock_data *lock_data, unsigned int context)
        return 0;
 }
 
-/**
- * If we get here, it means that the process has called DRM_IOCTL_LOCK
- * without calling DRM_IOCTL_UNLOCK.
- *
- * If the lock is not held, then let the signal proceed as usual.  If the lock
- * is held, then set the contended flag and keep the signal blocked.
- *
- * \param priv pointer to a drm_device structure.
- * \return one if the signal should be delivered normally, or zero if the
- * signal should be blocked.
- */
-static int drm_notifier(void *priv)
-{
-       struct drm_device *dev = priv;
-       struct drm_hw_lock *lock = dev->sigdata.lock;
-       unsigned int old, new, prev;
-
-       /* Allow signal delivery if lock isn't held */
-       if (!lock || !_DRM_LOCK_IS_HELD(lock->lock)
-           || _DRM_LOCKING_CONTEXT(lock->lock) != dev->sigdata.context)
-               return 1;
-
-       /* Otherwise, set flag to force call to
-          drmUnlock */
-       do {
-               old = lock->lock;
-               new = old | _DRM_LOCK_CONT;
-               prev = cmpxchg(&lock->lock, old, new);
-       } while (prev != old);
-       return 0;
-}
-
 /**
  * This function returns immediately and takes the hw lock
  * with the kernel context if it is free, otherwise it gets the highest priority when and if

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 8b5ce7c..f56cdce 100644 (file)
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -822,7 +822,6 @@ struct drm_device {
 
        struct drm_sg_mem *sg;  /**< Scatter gather memory */
        unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
-       sigset_t sigmask;
 
        struct {
                int context;

diff --git a/include/linux/sched.h b/include/linux/sched.h
index eeb5066..923ec1a 100644 (file)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1570,9 +1570,7 @@ struct task_struct {
 
        unsigned long sas_ss_sp;
        size_t sas_ss_size;
-       int (*notifier)(void *priv);
-       void *notifier_data;
-       sigset_t *notifier_mask;
+
        struct callback_head *task_works;
 
        struct audit_context *audit_context;
@@ -2476,9 +2474,6 @@ static inline int dequeue_signal_lock(struct task_struct *tsk, sigset_t *mask, s
        return ret;
 }
 
-extern void block_all_signals(int (*notifier)(void *priv), void *priv,
-                             sigset_t *mask);
-extern void unblock_all_signals(void);
 extern void release_task(struct task_struct * p);
 extern int send_sig_info(int, struct siginfo *, struct task_struct *);
 extern int force_sigsegv(int, struct task_struct *);

diff --git a/kernel/signal.c b/kernel/signal.c
index 0f6bbbe..f2cbd4e 100644 (file)
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -503,41 +503,6 @@ int unhandled_signal(struct task_struct *tsk, int sig)
        return !tsk->ptrace;
 }
 
-/*
- * Notify the system that a driver wants to block all signals for this
- * process, and wants to be notified if any signals at all were to be
- * sent/acted upon.  If the notifier routine returns non-zero, then the
- * signal will be acted upon after all.  If the notifier routine returns 0,
- * then then signal will be blocked.  Only one block per process is
- * allowed.  priv is a pointer to private data that the notifier routine
- * can use to determine if the signal should be blocked or not.
- */
-void
-block_all_signals(int (*notifier)(void *priv), void *priv, sigset_t *mask)
-{
-       unsigned long flags;
-
-       spin_lock_irqsave(&current->sighand->siglock, flags);
-       current->notifier_mask = mask;
-       current->notifier_data = priv;
-       current->notifier = notifier;
-       spin_unlock_irqrestore(&current->sighand->siglock, flags);
-}
-
-/* Notify the system that blocking has ended. */
-
-void
-unblock_all_signals(void)
-{
-       unsigned long flags;
-
-       spin_lock_irqsave(&current->sighand->siglock, flags);
-       current->notifier = NULL;
-       current->notifier_data = NULL;
-       recalc_sigpending();
-       spin_unlock_irqrestore(&current->sighand->siglock, flags);
-}
-
 static void collect_signal(int sig, struct sigpending *list, siginfo_t *info)
 {
        struct sigqueue *q, *first = NULL;
@@ -580,19 +545,8 @@ static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
 {
        int sig = next_signal(pending, mask);
 
-       if (sig) {
-               if (current->notifier) {
-                       if (sigismember(current->notifier_mask, sig)) {
-                               if (!(current->notifier)(current->notifier_data)) {
-                                       clear_thread_flag(TIF_SIGPENDING);
-                                       return 0;
-                               }
-                       }
-               }
-
+       if (sig)
                collect_signal(sig, pending, info);
-       }
-
        return sig;
 }
 
@@ -2483,9 +2437,6 @@ EXPORT_SYMBOL(force_sig);
 EXPORT_SYMBOL(send_sig);
 EXPORT_SYMBOL(send_sig_info);
 EXPORT_SYMBOL(sigprocmask);
-EXPORT_SYMBOL(block_all_signals);
-EXPORT_SYMBOL(unblock_all_signals);
-
 
 /*
  * System call entry points.