From patchwork Tue Jun 6 03:03:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Darren Hart X-Patchwork-Id: 9767989 X-Patchwork-Delegate: dvhart@infradead.org Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2AB4B6034B for ; Tue, 6 Jun 2017 03:03:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2026027F7F for ; Tue, 6 Jun 2017 03:03:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 14701283FD; Tue, 6 Jun 2017 03:03:20 +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=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A631827F7F for ; Tue, 6 Jun 2017 03:03:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751237AbdFFDDH (ORCPT ); Mon, 5 Jun 2017 23:03:07 -0400 Received: from bombadil.infradead.org ([65.50.211.133]:60547 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751194AbdFFDDG (ORCPT ); Mon, 5 Jun 2017 23:03:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Transfer-Encoding :Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=hFJsIZNVMXQntdgb8xTpFOCWAqYCgkbpC1sOb53UFHo=; b=Fut1AmFzx8XE8W7p/Fg30aNTaK 5rdnRwOlS6/hG89KJofElPJrNk3RBrJJjZZENrkmDch7Llpc9Dn3Jux46M8DMefqsuPIzXUR18J/x HnZb1VmEbmOF30/oB6/aUkmhFP7FHbHEbXYfqb6BCDPoT37l4oZQwzqgoTDDbrfUWXRsasTew5phQ 9DdXNTRMjD2V7jLIaWHjRMWJpn/VarGlBMdnvlDZrJBJvB4IDLz11faiEj6g7nsjoa3vLe7LynCCZ qY3b95FPD+D3ug5rfaOspkkr3UC3gPFldqyzjV9Msbs2RFrj46Eu40Zr8q/It0vs9IxO2LxcMPJgy wnBuPDyA==; Received: from dvhart by bombadil.infradead.org with local (Exim 4.87 #1 (Red Hat Linux)) id 1dI4lr-0008Fp-Ck; Tue, 06 Jun 2017 03:03:03 +0000 Date: Mon, 5 Jun 2017 20:03:02 -0700 From: Darren Hart To: =?utf-8?B?TWljaGHFgiBLxJlwaWXFhA==?= Cc: platform-driver-x86@vger.kernel.org, Andy Shevchenko , Andy Lutomirski , Andy Lutomirski , Mario Limonciello , Pali =?iso-8859-1?Q?Roh=E1r?= , Rafael Wysocki , linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org Subject: Re: [PATCH 09/16] platform/x86: wmi: Instantiate all devices before adding them Message-ID: <20170606030302.GC27270@fury> References: <20170601204339.GA1842@kmp-mobile.hq.kempniu.pl> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170601204339.GA1842@kmp-mobile.hq.kempniu.pl> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: platform-driver-x86-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jun 01, 2017 at 10:43:39PM +0200, Michał Kępień wrote: > I know I have probably started sounding like a broken record by now, but > I still have not seen any response (apart from the typos getting fixed) > to my comments on this patch which I posted in January 2016 [1]. > > None of the issues I found back then are really critical, but I did > point out a potential memory leak (granted, an unlikely one), so it > might be a good idea to at least take a second look before merging. > > [1] https://www.spinics.net/lists/platform-driver-x86/msg08201.html Thanks for being persistent, some good points in there. I'd like to just squash these into this patch (9/16), but I'll include them here for an ack from you and Andy L. that this is what you meant, and consistent with his intent/understanding: From 2512da1593574a66eb48d7105885e959b38db410 Mon Sep 17 00:00:00 2001 Message-Id: <2512da1593574a66eb48d7105885e959b38db410.1496717988.git.dvhart@infradead.org> From: "Darren Hart (VMware)" Date: Mon, 5 Jun 2017 19:54:03 -0700 Subject: [PATCH] =?UTF-8?q?platform/x86:=20wmi:=20Apply=20fixes=20per=20Mi?= =?UTF-8?q?cha=C5=82=20K=C4=99pie=C5=84=20(squash)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per: https://www.spinics.net/lists/platform-driver-x86/msg08201.html Eliminate the kfree of a wblock with a null bus now that we ignore duplicate GUIDs in parse_wdg. Move the out_free_pointer: label and kfree to the end of the function, clearly marking it as a return path. Rework the device_add loop at the end of parse_wdg to avoid leaking the wblock, and symmetrically call wmi_method_enable() if (debug_event). Signed-off-by: Darren Hart (VMware) --- drivers/platform/x86/wmi.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c index bfc0a3f..fbce876 100644 --- a/drivers/platform/x86/wmi.c +++ b/drivers/platform/x86/wmi.c @@ -863,14 +863,7 @@ static void wmi_free_devices(struct acpi_device *device) list_for_each_entry_safe(wblock, next, &wmi_block_list, list) { if (wblock->acpi_device == device) { list_del(&wblock->list); - if (wblock->dev.dev.bus) { - /* Device was initialized. */ - device_del(&wblock->dev.dev); - put_device(&wblock->dev.dev); - } else { - /* device_initialize was not called. */ - kfree(wblock); - } + device_unregister(&wblock->dev.dev); } } } @@ -903,9 +896,9 @@ static bool guid_already_parsed(struct acpi_device *device, static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) { struct acpi_buffer out = {ACPI_ALLOCATE_BUFFER, NULL}; - union acpi_object *obj; const struct guid_block *gblock; struct wmi_block *wblock, *next; + union acpi_object *obj; acpi_status status; int retval = 0; u32 i, total; @@ -958,24 +951,27 @@ static int parse_wdg(struct device *wmi_bus_dev, struct acpi_device *device) } } -out_free_pointer: - kfree(out.pointer); - /* * Now that all of the devices are created, add them to the * device tree and probe subdrivers. */ list_for_each_entry_safe(wblock, next, &wmi_block_list, list) { - if (wblock->acpi_device == device) { - if (device_add(&wblock->dev.dev) != 0) { - dev_err(wmi_bus_dev, - "failed to register %pULL\n", - wblock->gblock.guid); - list_del(&wblock->list); - } + if (wblock->acpi_device != device) + continue; + + retval = device_add(&wblock->dev.dev); + if (retval) { + dev_err(wmi_bus_dev, "failed to register %pULL\n", + wblock->gblock.guid); + if (debug_event) + wmi_method_enable(wblock, 0); + list_del(&wblock->list); + put_device(&wblock->dev.dev); } } +out_free_pointer: + kfree(out.pointer); return retval; }