Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2e01fabe authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Linus Torvalds
Browse files

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: default avatarOleg Nesterov <oleg@redhat.com>
Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: default avatarDave Airlie <airlied@redhat.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 4f05028f

drivers/gpu/drm/drm_lock.c

+0 −41
Original line number Diff line number Diff line
@@ -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

include/drm/drmP.h

+0 −1
Original line number Diff line number Diff line
@@ -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;

include/linux/sched.h

+1 −6
Original line number Diff line number Diff line
@@ -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 *);

kernel/signal.c

+1 −50
Original line number Diff line number Diff line
@@ -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.