From patchwork Thu Aug 13 16:53:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 7009441 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AC1589F344 for ; Thu, 13 Aug 2015 16:53:46 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BEC8F20364 for ; Thu, 13 Aug 2015 16:53:45 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id D554320465 for ; Thu, 13 Aug 2015 16:53:44 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E534188FAD; Thu, 13 Aug 2015 09:53:43 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by gabe.freedesktop.org (Postfix) with ESMTPS id B44B088FAD for ; Thu, 13 Aug 2015 09:53:42 -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 B1B9937251F; Thu, 13 Aug 2015 16:53:41 +0000 (UTC) Received: from shalem.localdomain.com (vpn1-5-252.ams2.redhat.com [10.36.5.252]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7DGrclY012960; Thu, 13 Aug 2015 12:53:39 -0400 From: Hans de Goede To: "Rafael J. Wysocki" Date: Thu, 13 Aug 2015 18:53:37 +0200 Message-Id: <1439484817-2888-1-git-send-email-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 Cc: Aaron Lu , Hans de Goede , Daniel Vetter , intel-gfx , linux-acpi@vger.kernel.org, Sedat Dilek , Zhang Rui , Linus Torvalds , Len Brown Subject: [Intel-gfx] [PATCH] ACPI / video: Fix circular lock dependency issue in the video-detect code X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Before this commit, the following would happen: a) acpi_video_get_backlight_type() gets called b) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type() c) acpi_video_init_backlight_type() locks its function static init_mutex d) acpi_video_init_backlight_type() calls backlight_register_notifier() e) backlight_register_notifier() takes its notifier-chain lock And when the backlight notifier chain gets called we've: 1) blocking_notifier_call_chain() gets called 2) blocking_notifier_call_chain() takes the notifier-chain lock 3) blocking_notifier_call_chain() calls acpi_video_backlight_notify() 4) acpi_video_backlight_notify() calls acpi_video_get_backlight_type() 5) acpi_video_get_backlight_type() calls acpi_video_init_backlight_type() 6) acpi_video_init_backlight_type() locks its function static init_mutex So in the first call sequence we have: a) init_mutex gets locked b) notifier-chain gets locked and in the second call sequence we have: 1) notifier-chain gets locked 2) init_mutex gets locked And we've a circular locking dependency. This specific locking dependency is fixable without using the big hammer otherwise known as a workqueue, but further analysis shows a similar problem with the backlight notifier chain lock vs register_count_mutex from drivers/acpi/acpi_video.c, and fixing that becomes problematic. So this commit simply fixes this with the big hammer, performance wise this is a non issue as we expect the work to get scheduled exactly zero or one times during normal system use. Signed-off-by: Hans de Goede --- drivers/acpi/video_detect.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c index 815f75e..2922f1f 100644 --- a/drivers/acpi/video_detect.c +++ b/drivers/acpi/video_detect.c @@ -32,6 +32,7 @@ #include #include #include +#include #include ACPI_MODULE_NAME("video"); @@ -41,6 +42,7 @@ void acpi_video_unregister_backlight(void); static bool backlight_notifier_registered; static struct notifier_block backlight_nb; +static struct work_struct backlight_notify_work; static enum acpi_backlight_type acpi_backlight_cmdline = acpi_backlight_undef; static enum acpi_backlight_type acpi_backlight_dmi = acpi_backlight_undef; @@ -262,6 +264,13 @@ static const struct dmi_system_id video_detect_dmi_table[] = { { }, }; +/* This uses a workqueue to avoid various locking ordering issues */ +static void acpi_video_backlight_notify_work(struct work_struct *work) +{ + if (acpi_video_get_backlight_type() != acpi_backlight_video) + acpi_video_unregister_backlight(); +} + static int acpi_video_backlight_notify(struct notifier_block *nb, unsigned long val, void *bd) { @@ -269,9 +278,8 @@ static int acpi_video_backlight_notify(struct notifier_block *nb, /* A raw bl registering may change video -> native */ if (backlight->props.type == BACKLIGHT_RAW && - val == BACKLIGHT_REGISTERED && - acpi_video_get_backlight_type() != acpi_backlight_video) - acpi_video_unregister_backlight(); + val == BACKLIGHT_REGISTERED) + schedule_work(&backlight_notify_work); return NOTIFY_OK; } @@ -304,6 +312,8 @@ enum acpi_backlight_type acpi_video_get_backlight_type(void) acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, find_video, NULL, &video_caps, NULL); + INIT_WORK(&backlight_notify_work, + acpi_video_backlight_notify_work); backlight_nb.notifier_call = acpi_video_backlight_notify; backlight_nb.priority = 0; if (backlight_register_notifier(&backlight_nb) == 0)