From patchwork Mon May 6 07:45:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 10930673 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 79EC91395 for ; Mon, 6 May 2019 07:46:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6875D28686 for ; Mon, 6 May 2019 07:46:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5AF3F2868D; Mon, 6 May 2019 07:46:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id ACF2928686 for ; Mon, 6 May 2019 07:46:03 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8BCAE89115; Mon, 6 May 2019 07:46:02 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ed1-x541.google.com (mail-ed1-x541.google.com [IPv6:2a00:1450:4864:20::541]) by gabe.freedesktop.org (Postfix) with ESMTPS id 169DD89115 for ; Mon, 6 May 2019 07:46:01 +0000 (UTC) Received: by mail-ed1-x541.google.com with SMTP id f37so14216671edb.13 for ; Mon, 06 May 2019 00:46:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=04n4bi/0il+bPmWqsIQ2e+x6fvuMJP2MziNsJbLuWN4=; b=HdLCuP4CK8kdsGDTp7GKoxgw/Gkt611I8ZC6e3bqbHsYuAoy4UUDG9XsWs+RJ02EHU TBUseOCUf7evM2HA3nYVHQEkrL0ze8sabLIU+zmMlCNSM6Kb920SsEWCAB5SmnA129aL JhZdUmk0ncH2prKamudOwBGx78AgRKpDl3Ik56ff1hgw/jlyVMn8DQi53BU4QH1Uk1rm it9vh5GefyVp85VXOtOK4R6/xik0fg1bdlnZCwRJUXGs79vq7JRzTeqdZSoZlgmilJ+L DsMqNa+S79mJSZtxPxDEMlm8uKdhAzY+l2iVXMKWoOwoZRQu4NxtlWOiHQKf6awXb0hw CTmw== X-Gm-Message-State: APjAAAXrSmvaLsiop4IZ4J+mMJ4acW2Y9XJSPU+xp+PTsE0jxLbZp9Wh MdQT38VxBaD2KO9G8ZD4MevsVCpe6Ro= X-Google-Smtp-Source: APXvYqzon9mPPFB9xFLuZ56pH3fApyh6Ek73kjZdssOe6Nvobk9Y3fKvc1cQgXfVibqPEzhGE6bNeA== X-Received: by 2002:a50:8bbb:: with SMTP id m56mr24893171edm.230.1557128759478; Mon, 06 May 2019 00:45:59 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:569e:0:3106:d637:d723:e855]) by smtp.gmail.com with ESMTPSA id z11sm1451872ejo.45.2019.05.06.00.45.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 06 May 2019 00:45:58 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Mon, 6 May 2019 09:45:53 +0200 Message-Id: <20190506074553.21464-1-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190502141643.21080-1-daniel.vetter@ffwll.ch> References: <20190502141643.21080-1-daniel.vetter@ffwll.ch> MIME-Version: 1.0 X-Mailman-Original-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=04n4bi/0il+bPmWqsIQ2e+x6fvuMJP2MziNsJbLuWN4=; b=g+Rj6lZOq58m6EpuhkaGKAgELBYFqZayO0UVs6XuXjcKuZ9wZW696L0FAvqpvsC3Sz 5LdY6QKVhDTtVGSkx4p7OhAoLS9tQxxHKqNASJ6XRM523im0HjxD4QMpKJJbxL6qls2A COWjAYliGJcM4AvajlOKV2UoIpFu9r95F/qBI= Subject: [Intel-gfx] [PATCH] RFC: console: hack up console_lock more v2 X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Petr Mladek , John Ogness , Peter Zijlstra , Daniel Vetter , Will Deacon , linux-kernel@vger.kernel.org, Steven Rostedt , Sergey Senozhatsky , Ingo Molnar , Daniel Vetter Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP console_trylock, called from within printk, can be called from pretty much anywhere. Including try_to_wake_up. Note that this isn't common, usually the box is in pretty bad shape at that point already. But it really doesn't help when then lockdep jumps in and spams the logs, potentially obscuring the real backtrace we're really interested in. One case I've seen (slightly simplified backtrace): Call Trace: console_trylock+0xe/0x60 vprintk_emit+0xf1/0x320 printk+0x4d/0x69 __warn_printk+0x46/0x90 native_smp_send_reschedule+0x2f/0x40 check_preempt_curr+0x81/0xa0 ttwu_do_wakeup+0x14/0x220 try_to_wake_up+0x218/0x5f0 pollwake+0x6f/0x90 credit_entropy_bits+0x204/0x310 add_interrupt_randomness+0x18f/0x210 handle_irq+0x67/0x160 do_IRQ+0x5e/0x130 common_interrupt+0xf/0xf This alone isn't a problem, but the spinlock in the semaphore is also still held while waking up waiters (up() -> __up() -> try_to_wake_up() callchain), which then closes the runqueue vs. semaphore.lock loop, and upsets lockdep, which issues a circular locking splat to dmesg. Worse it upsets developers, since we don't want to spam dmesg with clutter when the machine is dying already. Fix this by creating a prinkt_safe_up() which calls wake_up_process outside of the spinlock. This isn't correct in full generality, but good enough for console_lock: - console_lock doesn't use interruptible or killable or timeout down() calls, hence an up() is the only thing that can wake up a process. Hence the process can't get woken and killed and reaped while we try to wake it up too. - semaphore.c always updates the waiter list while under the spinlock, so there's no other races. Specifically another process that races with a quick console_lock/unlock while we've dropped the spinlock already won't see our own waiter. Also cc'ing John Ogness since perhaps his printk rework fixes this all properly. Signed-off-by: Daniel Vetter Cc: Peter Zijlstra Cc: Ingo Molnar Cc: Will Deacon Cc: Petr Mladek Cc: Sergey Senozhatsky Cc: Steven Rostedt Cc: Daniel Vetter Cc: John Ogness Cc: linux-kernel@vger.kernel.org --- v2: My first attempt at patching down_trylock didn't work (thanks to Petr for pointing out), now try to have a special up() that breaks the cycle. -Daniel --- include/linux/semaphore.h | 1 + kernel/locking/semaphore.c | 31 +++++++++++++++++++++++++++++++ kernel/printk/printk.c | 2 +- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/include/linux/semaphore.h b/include/linux/semaphore.h index 11c86fbfeb98..7e839c72809d 100644 --- a/include/linux/semaphore.h +++ b/include/linux/semaphore.h @@ -41,6 +41,7 @@ extern int __must_check down_interruptible(struct semaphore *sem); extern int __must_check down_killable(struct semaphore *sem); extern int __must_check down_trylock(struct semaphore *sem); extern int __must_check down_timeout(struct semaphore *sem, long jiffies); +extern void printk_safe_up(struct semaphore *sem); extern void up(struct semaphore *sem); #endif /* __LINUX_SEMAPHORE_H */ diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c index 561acdd39960..84675993dc59 100644 --- a/kernel/locking/semaphore.c +++ b/kernel/locking/semaphore.c @@ -197,6 +197,37 @@ struct semaphore_waiter { bool up; }; +/** + * printk_safe_up - release the semaphore in console_unlock + * @sem: the semaphore to release + * + * Release the semaphore. Unlike mutexes, up() may be called from any + * context and even by tasks which have never called down(). + * + * NOTE: This is a special version of up() for console_unlock only. It is only + * safe if there are no killable, interruptible or timing out down() calls. + */ +void printk_safe_up(struct semaphore *sem) +{ + unsigned long flags; + struct semaphore_waiter *waiter = NULL; + + raw_spin_lock_irqsave(&sem->lock, flags); + if (likely(list_empty(&sem->wait_list))) { + sem->count++; + } else { + waiter = list_first_entry(&sem->wait_list, + struct semaphore_waiter, list); + list_del(&waiter->list); + waiter->up = true; + } + raw_spin_unlock_irqrestore(&sem->lock, flags); + + if (waiter) + wake_up_process(waiter->task); +} +EXPORT_SYMBOL(printk_safe_up); + /* * Because this function is inlined, the 'state' parameter will be * constant, and thus optimised away by the compiler. Likewise the diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 02ca827b8fac..62303929afda 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -234,7 +234,7 @@ static void __up_console_sem(unsigned long ip) mutex_release(&console_lock_dep_map, 1, ip); printk_safe_enter_irqsave(flags); - up(&console_sem); + printk_safe_up(&console_sem); printk_safe_exit_irqrestore(flags); } #define up_console_sem() __up_console_sem(_RET_IP_)