From patchwork Mon Oct 28 06:01:12 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gu Zheng X-Patchwork-Id: 3100651 Return-Path: X-Original-To: patchwork-linux-fbdev@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5B2169F432 for ; Mon, 28 Oct 2013 06:07:27 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 93C55202C8 for ; Mon, 28 Oct 2013 06:07:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9FC97201B5 for ; Mon, 28 Oct 2013 06:07:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752327Ab3J1GHS (ORCPT ); Mon, 28 Oct 2013 02:07:18 -0400 Received: from cn.fujitsu.com ([222.73.24.84]:16083 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1752233Ab3J1GHR (ORCPT ); Mon, 28 Oct 2013 02:07:17 -0400 X-IronPort-AV: E=Sophos;i="4.93,584,1378828800"; d="scan'208";a="8875719" Received: from unknown (HELO tang.cn.fujitsu.com) ([10.167.250.3]) by song.cn.fujitsu.com with ESMTP; 28 Oct 2013 14:03:53 +0800 Received: from fnstmail02.fnst.cn.fujitsu.com (tang.cn.fujitsu.com [127.0.0.1]) by tang.cn.fujitsu.com (8.14.3/8.13.1) with ESMTP id r9S678V0028616; Mon, 28 Oct 2013 14:07:09 +0800 Received: from [10.167.226.100] ([10.167.226.100]) by fnstmail02.fnst.cn.fujitsu.com (Lotus Domino Release 8.5.3) with ESMTP id 2013102814053133-2569464 ; Mon, 28 Oct 2013 14:05:31 +0800 Message-ID: <526DFDA8.7010806@cn.fujitsu.com> Date: Mon, 28 Oct 2013 14:01:12 +0800 From: Gu Zheng User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: Jean-Christophe PLAGNIOL-VILLARD , Tomi Valkeinen CC: Linux Fbdev development list , linux-kernel Subject: [RFC PATCH RESEND] fb: reorder the lock sequence to fix a potential lockdep X-MIMETrack: Itemize by SMTP Server on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/10/28 14:05:31, Serialize by Router on mailserver/fnst(Release 8.5.3|September 15, 2011) at 2013/10/28 14:05:36, Serialize complete at 2013/10/28 14:05:36 Sender: linux-fbdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fbdev@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 Following commits: 50e244cc79 fb: rework locking to fix lock ordering on takeover e93a9a8687 fb: Yet another band-aid for fixing lockdep mess 054430e773 fbcon: fix locking harder reworked locking to fix related lock ordering on takeover, and introduced console_lock into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock) is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to a potential deadlock as following: [ 601.079000] ====================================================== [ 601.079000] [ INFO: possible circular locking dependency detected ] [ 601.079000] 3.11.0 #189 Not tainted [ 601.079000] ------------------------------------------------------- [ 601.079000] kworker/0:3/619 is trying to acquire lock: [ 601.079000] (&fb_info->lock){+.+.+.}, at: [] lock_fb_info+0x26/0x60 [ 601.079000] but task is already holding lock: [ 601.079000] (console_lock){+.+.+.}, at: [] console_callback+0x13/0x160 [ 601.079000] which lock already depends on the new lock. [ 601.079000] the existing dependency chain (in reverse order) is: [ 601.079000] -> #1 (console_lock){+.+.+.}: [ 601.079000] [] lock_acquire+0xa1/0x140 [ 601.079000] [] console_lock+0x77/0x80 [ 601.079000] [] register_framebuffer+0x1d8/0x320 [ 601.079000] [] efifb_probe+0x408/0x48f [ 601.079000] [] platform_drv_probe+0x43/0x80 [ 601.079000] [] driver_probe_device+0x8b/0x390 [ 601.079000] [] __driver_attach+0xab/0xb0 [ 601.079000] [] bus_for_each_dev+0x5d/0xa0 [ 601.079000] [] driver_attach+0x1e/0x20 [ 601.079000] [] bus_add_driver+0x117/0x290 [ 601.079000] [] driver_register+0x7a/0x170 [ 601.079000] [] __platform_driver_register+0x4a/0x50 [ 601.079000] [] platform_driver_probe+0x1d/0xb0 [ 601.079000] [] efifb_init+0x273/0x292 [ 601.079000] [] do_one_initcall+0x102/0x1c0 [ 601.079000] [] kernel_init_freeable+0x15d/0x1ef [ 601.079000] [] kernel_init+0xe/0xf0 [ 601.079000] [] ret_from_fork+0x7c/0xb0 [ 601.079000] -> #0 (&fb_info->lock){+.+.+.}: [ 601.079000] [] __lock_acquire+0x1e18/0x1f10 [ 601.079000] [] lock_acquire+0xa1/0x140 [ 601.079000] [] mutex_lock_nested+0x7a/0x3b0 [ 601.079000] [] lock_fb_info+0x26/0x60 [ 601.079000] [] fbcon_blank+0x29b/0x2e0 [ 601.079000] [] do_blank_screen+0x1d8/0x280 [ 601.079000] [] console_callback+0x64/0x160 [ 601.079000] [] process_one_work+0x1f5/0x540 [ 601.079000] [] worker_thread+0x11c/0x370 [ 601.079000] [] kthread+0xed/0x100 [ 601.079000] [] ret_from_fork+0x7c/0xb0 [ 601.079000] other info that might help us debug this: [ 601.079000] Possible unsafe locking scenario: [ 601.079000] CPU0 CPU1 [ 601.079000] ---- ---- [ 601.079000] lock(console_lock); [ 601.079000] lock(&fb_info->lock); [ 601.079000] lock(console_lock); [ 601.079000] lock(&fb_info->lock); [ 601.079000] *** DEADLOCK *** so we reorder the lock sequence the same as it in console_callback() to avoid this issue. Not very sure this change is suitable, any comments is welcome. Signed-off-by: Gu Zheng --- drivers/video/fbmem.c | 50 +++++++++++++++++++++++++++++++----------------- 1 files changed, 32 insertions(+), 18 deletions(-) diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c index dacaf74..010d191 100644 --- a/drivers/video/fbmem.c +++ b/drivers/video/fbmem.c @@ -1108,14 +1108,16 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOPUT_VSCREENINFO: if (copy_from_user(&var, argp, sizeof(var))) return -EFAULT; - if (!lock_fb_info(info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(info)) { + console_unlock(); + return -ENODEV; + } info->flags |= FBINFO_MISC_USEREVENT; ret = fb_set_var(info, &var); info->flags &= ~FBINFO_MISC_USEREVENT; - console_unlock(); unlock_fb_info(info); + console_unlock(); if (!ret && copy_to_user(argp, &var, sizeof(var))) ret = -EFAULT; break; @@ -1144,12 +1146,14 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, case FBIOPAN_DISPLAY: if (copy_from_user(&var, argp, sizeof(var))) return -EFAULT; - if (!lock_fb_info(info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(info)) { + console_unlock(); + return -ENODEV; + } ret = fb_pan_display(info, &var); - console_unlock(); unlock_fb_info(info); + console_unlock(); if (ret == 0 && copy_to_user(argp, &var, sizeof(var))) return -EFAULT; break; @@ -1184,23 +1188,27 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd, break; } event.data = &con2fb; - if (!lock_fb_info(info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(info)) { + console_unlock(); + return -ENODEV; + } event.info = info; ret = fb_notifier_call_chain(FB_EVENT_SET_CONSOLE_MAP, &event); - console_unlock(); unlock_fb_info(info); + console_unlock(); break; case FBIOBLANK: - if (!lock_fb_info(info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(info)) { + console_unlock(); + return -ENODEV; + } info->flags |= FBINFO_MISC_USEREVENT; ret = fb_blank(info, arg); info->flags &= ~FBINFO_MISC_USEREVENT; - console_unlock(); unlock_fb_info(info); + console_unlock(); break; default: if (!lock_fb_info(info)) @@ -1660,12 +1668,15 @@ static int do_register_framebuffer(struct fb_info *fb_info) registered_fb[i] = fb_info; event.info = fb_info; - if (!lock_fb_info(fb_info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(fb_info)) { + console_unlock(); + return -ENODEV; + } + fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event); - console_unlock(); unlock_fb_info(fb_info); + console_unlock(); return 0; } @@ -1678,13 +1689,16 @@ static int do_unregister_framebuffer(struct fb_info *fb_info) if (i < 0 || i >= FB_MAX || registered_fb[i] != fb_info) return -EINVAL; - if (!lock_fb_info(fb_info)) - return -ENODEV; console_lock(); + if (!lock_fb_info(fb_info)) { + console_unlock(); + return -ENODEV; + } + event.info = fb_info; ret = fb_notifier_call_chain(FB_EVENT_FB_UNBIND, &event); - console_unlock(); unlock_fb_info(fb_info); + console_unlock(); if (ret) return -EINVAL;