From patchwork Mon May 25 16:50:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oleg Nesterov X-Patchwork-Id: 6477721 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id F3DBEC0020 for ; Tue, 26 May 2015 03:56:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 10E972042B for ; Tue, 26 May 2015 03:56:46 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id DAC1B20439 for ; Tue, 26 May 2015 03:56:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 456216E5C1; Mon, 25 May 2015 20:56:44 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTP id D3BB36E235 for ; Mon, 25 May 2015 09:51:17 -0700 (PDT) Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 1040AA10BD; Mon, 25 May 2015 16:51:17 +0000 (UTC) Received: from tranklukator.brq.redhat.com (dhcp-1-208.brq.redhat.com [10.34.1.208]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id t4PGpDBt027836; Mon, 25 May 2015 12:51:14 -0400 Received: by tranklukator.brq.redhat.com (nbSMTP-1.00) for uid 500 oleg@redhat.com; Mon, 25 May 2015 18:50:35 +0200 (CEST) Date: Mon, 25 May 2015 18:50:32 +0200 From: Oleg Nesterov To: Richard Weinberger Subject: Re: block_all_signals() usage in DRM Message-ID: <20150525165032.GB32370@redhat.com> References: <556338DA.6000404@nod.at> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <556338DA.6000404@nod.at> User-Agent: Mutt/1.5.18 (2008-05-17) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Mailman-Approved-At: Mon, 25 May 2015 20:56:36 -0700 Cc: "linux-kernel@vger.kernel.org" , dri-devel@lists.freedesktop.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP AAAAOn 05/25, Richard Weinberger wrote: > > Is this functionality still in use/needed? All I can say it doesn't work. > Otherwise we could get rid of block_all_signals() and unpuzzle the signaling > code a bit. :-) Yes. I do not even remember when I reported this the first time. Perhaps more than 10 years ago. See the last attempt in 2011: https://lkml.org/lkml/2011/7/12/263 I copied this email below. Dave. Lets finally kill this horror? I am going to send a patch unless you stop me ;) Oleg. ------------------------------------------------------------------------------- I tried many times to ask about the supposed behaviour of block_all_signals() in drm, but it seems nobody can answer. So I am going to send the patch which simply removes block_all_signals() and friends. There are numeruous problems with this interace, I can't even enumerate them. But I think that it is enough to mention that block_all_signals() simply can not work. AT ALL. I am wondering, was it ever tested and how. So. ioctl()->drm_lock() "blocks" the stop signals. Probably to ensure the task can't be stopped until it does DRM_IOCTL_UNLOCK. And what does this mean? Yes, the task won't stop if it receives, say, SIGTSTP. But! Instead it will loop forever in kernel mode until it receives another unblocked/non-ignored signal which should be numerically less than SIGSTOP. Why do we need this? Once again. block_all_signals(SIGTSTP) only means that the caller will burn cpu instead of sleeping in TASK_STOPPED after ^Z. What is the point? And once again, there are other problems. For example, even if block_all_signals() actually blocked SIGSTOP/etc, this can not help if the caller is multithreaded. I strongly believe block_all_signals() should die. Given that it doesn't work, could somebody please explain me what will be broken? Just in case... Please look at the debugging patch below. With this patch, $ perl -le 'syscall 157,666 and die $!; sleep 1, print while ++$_' 1 2 3 ^Z Hang. So it does react to ^Z anyway, just it is looping in the endless loop in the kernel. It can only look as if ^Z is ignored, because obviously bash doesn't see it stopped. Now lets look at drm_notifier(). If it returns 0 it does: /* Otherwise, set flag to force call to drmUnlock */ drmUnlock? grep shows nothing... do { old = s->lock->lock; new = old | _DRM_LOCK_CONT; prev = cmpxchg(&s->lock->lock, old, new); } while (prev != old); return 0; OK. So, if block_all_signals() makes any sense, it seems that this is only because we add _DRM_LOCK_CONT. Who checks _DRM_LOCK_CONT? _DRM_LOCK_IS_CONT(), but it has no users. Hmm. Looks like via_release_futex() is the only user, but it doesn't look as "force call to drmUnlock" and it is CONFIG_DRM_VIA only. I am totally confused. But block_all_signals() should die anyway. We can probably implement something like 'i-am-going-to-stop' or even 'can-i-stop' per-thread notifiers, although this all looks like the user-space problem to me (yes, I know absolutely nothing about drm/etc). If nothing else. We can change drm_lock/drm_unlock to literally block/unblock SIGSTOP/etc (or perhaps we only should worry about the signals from tty?). This is the awful hack and this can't work with the multithreaded tasks too, but still it is better than what we have now. Oleg. --- a/kernel/sys.c~ 2011-06-16 20:12:18.000000000 +0200 +++ b/kernel/sys.c 2011-07-12 16:24:50.000000000 +0200 @@ -1614,6 +1614,11 @@ SYSCALL_DEFINE1(umask, int, mask) return mask; } +static int notifier(void *arg) +{ + return 0; +} + SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, unsigned long, arg4, unsigned long, arg5) { @@ -1627,6 +1632,13 @@ SYSCALL_DEFINE5(prctl, int, option, unsi error = 0; switch (option) { + case 666: { + sigset_t *pmask = kmalloc(sizeof(*pmask), GFP_KERNEL); + siginitset(pmask, sigmask(SIGTSTP)); + block_all_signals(notifier, NULL, pmask); + break; + } + case PR_SET_PDEATHSIG: if (!valid_signal(arg2)) { error = -EINVAL;