[{"id":3682921,"web_url":"http://patchwork.ozlabs.org/comment/3682921/","msgid":"<DI472FJ8UOG4.1JGTQK7W1RAX@kernel.org>","list_archive_url":null,"date":"2026-04-27T19:46:20","subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","submitter":{"id":89037,"url":"http://patchwork.ozlabs.org/api/people/89037/","name":"Danilo Krummrich","email":"dakr@kernel.org"},"content":"On Mon Apr 27, 2026 at 9:31 PM CEST, Samiullah Khawaja wrote:\n> Fixes: cb3d1049f4ea (\"driver core: generalize driver_override in struct device\")\n\nI don't think anything is wrong with this commit, and it seems unrelated.\n\n> Fixes: 10a4206a2401 (\"PCI: use generic driver_override infrastructure\")\n\nI'm also not sure that this one contains the root cause, despite revealing the\nactual issue, more below.\n\n> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>\n> ---\n>  drivers/pci/pci-driver.c | 5 +++++\n>  1 file changed, 5 insertions(+)\n>\n> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c\n> index d10ece0889f0..5f453213b8c5 100644\n> --- a/drivers/pci/pci-driver.c\n> +++ b/drivers/pci/pci-driver.c\n> @@ -215,6 +215,11 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,\n>  \t\tpdev->subsystem_device = subdevice;\n>  \t\tpdev->class = class;\n>  \n> +\t\t/*\n> +\t\t * Initialize the embedded struct device driver_override lock to\n> +\t\t * avoid the lockdep errors.\n> +\t\t */\n> +\t\tspin_lock_init(&pdev->dev.driver_override.lock);\n\nCan't we just call device_initialize() and set pdev->dev.release to a new\nfunction that just calls kfree()?\n\nThis way nothing of that kind can ever happen again; it is hard to predict that\na device is used without ever being initialized.\n\n>  \t\tif (pci_match_device(pdrv, pdev))\n>  \t\t\tretval = -EEXIST;\n>  \n>\n> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731\n> -- \n> 2.54.0.545.g6539524ca2-goog","headers":{"Return-Path":"\n <linux-pci+bounces-53247-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=lZniO/DM;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=104.64.211.4; helo=sin.lore.kernel.org;\n envelope-from=linux-pci+bounces-53247-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"lZniO/DM\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sin.lore.kernel.org (sin.lore.kernel.org [104.64.211.4])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g4Db96G82z1xvV\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 28 Apr 2026 05:46:29 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sin.lore.kernel.org (Postfix) with ESMTP id E351E30060AF\n\tfor <incoming@patchwork.ozlabs.org>; Mon, 27 Apr 2026 19:46:26 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id B7CCD390CBE;\n\tMon, 27 Apr 2026 19:46:23 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 9554A266581;\n\tMon, 27 Apr 2026 19:46:23 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 9830AC19425;\n\tMon, 27 Apr 2026 19:46:21 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777319183; cv=none;\n b=q0bU35qr1UM9E2vAbQK9aeJLmpYdv2uRSkL+wH0xQMbXohP25ztPQQVgmtCrfd8FIvgZx2hDbW3E/d4Rw0NouCnifDcMWY6qmQ6fwNqMd1N5tLadK63nR3LnfcIe0f8d18o/0zHNXmoVJjozevWnyKOUo7QnsXF5S6rhwY6s4YY=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777319183; c=relaxed/simple;\n\tbh=v1R/95ouEPrTFF8ffv/0xVsBHMFb9M3cHebK5elKK9k=;\n\th=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From:\n\t References:In-Reply-To;\n b=FkhqHKYt0A9Z+hhfKQUKqwlm2efKFHdhvKQGRlWp6VYwhcy1RSAcz7gozrUuXakB20QtxRvQBGtuhHzcI4Sel2PbPZmAZOQOOYG8aASe/eZEM31B1glRe+4ht5eMwZKHY7AaW0mJCfIU+GKyxarrW0eM3njA/nqgt9WDWCdxY5Q=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=lZniO/DM; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1777319183;\n\tbh=v1R/95ouEPrTFF8ffv/0xVsBHMFb9M3cHebK5elKK9k=;\n\th=Date:Subject:Cc:To:From:References:In-Reply-To:From;\n\tb=lZniO/DMfgD5G8+6N29cnq/nmt98m6TdZx5H4DNmzggok+EqMMyiR5jevfYwXiLMw\n\t JEYBUJdLYNj+2G4PbSqz94oB5O6Rf3wL4gCiPJGB85zZE2cmWsI5fTNNfkcRaH+y91\n\t +8N1/7Vt3GswpN8lfVDduLaimw6wPGg87n5uyWWuXgyBf41qMowS+e4iPARzjNnouW\n\t s/yOIKI+cFhk5CuNNMMbkbSZxiK4c1yiwtJxNj4IaupHCwAm0Jd8yzPzb1fmXPiiSb\n\t kFV+kgC7Ck+VTdKbqWLLcyh6Lef7mrClD46HmOEZsZglI32nwVyNORPIEIUvOSM5fG\n\t L4VXvn+sc98Uw==","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"<linux-pci.vger.kernel.org>","List-Subscribe":"<mailto:linux-pci+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pci+unsubscribe@vger.kernel.org>","Mime-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=UTF-8","Date":"Mon, 27 Apr 2026 21:46:20 +0200","Message-Id":"<DI472FJ8UOG4.1JGTQK7W1RAX@kernel.org>","Subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","Cc":"\"Bjorn Helgaas\" <bhelgaas@google.com>, \"Gui-Dong Han\"\n <hanguidong02@gmail.com>, \"Greg Kroah-Hartman\"\n <gregkh@linuxfoundation.org>, \"Alex Williamson\" <alex@shazbot.org>, \"open\n list:PCI SUBSYSTEM\" <linux-pci@vger.kernel.org>, \"open list\"\n <linux-kernel@vger.kernel.org>, <dmatlack@google.com>","To":"\"Samiullah Khawaja\" <skhawaja@google.com>","From":"\"Danilo Krummrich\" <dakr@kernel.org>","References":"<20260427193139.2109938-1-skhawaja@google.com>","In-Reply-To":"<20260427193139.2109938-1-skhawaja@google.com>"}},{"id":3682974,"web_url":"http://patchwork.ozlabs.org/comment/3682974/","msgid":"<ae-_EmuuoQTqD6wg@google.com>","list_archive_url":null,"date":"2026-04-27T20:36:02","subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","submitter":{"id":91295,"url":"http://patchwork.ozlabs.org/api/people/91295/","name":"Samiullah Khawaja","email":"skhawaja@google.com"},"content":"On Mon, Apr 27, 2026 at 09:46:20PM +0200, Danilo Krummrich wrote:\n>On Mon Apr 27, 2026 at 9:31 PM CEST, Samiullah Khawaja wrote:\n>> Fixes: cb3d1049f4ea (\"driver core: generalize driver_override in struct device\")\n>\n>I don't think anything is wrong with this commit, and it seems unrelated.\n\nI will remove this one.\n>\n>> Fixes: 10a4206a2401 (\"PCI: use generic driver_override infrastructure\")\n>\n>I'm also not sure that this one contains the root cause, despite revealing the\n>actual issue, more below.\n\nI see your point. I understand the actual issue is that the temporary\npci_dev struct was not init and that was added in 2014. I added this\nsince this is where the regression happened.\n>\n>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>\n>> ---\n>>  drivers/pci/pci-driver.c | 5 +++++\n>>  1 file changed, 5 insertions(+)\n>>\n>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c\n>> index d10ece0889f0..5f453213b8c5 100644\n>> --- a/drivers/pci/pci-driver.c\n>> +++ b/drivers/pci/pci-driver.c\n>> @@ -215,6 +215,11 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,\n>>  \t\tpdev->subsystem_device = subdevice;\n>>  \t\tpdev->class = class;\n>>\n>> +\t\t/*\n>> +\t\t * Initialize the embedded struct device driver_override lock to\n>> +\t\t * avoid the lockdep errors.\n>> +\t\t */\n>> +\t\tspin_lock_init(&pdev->dev.driver_override.lock);\n>\n>Can't we just call device_initialize() and set pdev->dev.release to a new\n>function that just calls kfree()?\n>\n>This way nothing of that kind can ever happen again; it is hard to predict that\n>a device is used without ever being initialized.\n\nAgreed.\n\nyes we can set the release function and call device_initialize().\n\nShould I send v2 with something like following:\n\ndiff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c\nindex d10ece0889f0..634b4311bf5d 100644\n--- a/drivers/pci/pci-driver.c\n+++ b/drivers/pci/pci-driver.c\n@@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,\n         return NULL;\n  }\n\n+static void _release_temp_device(struct device *dev)\n+{\n+       kfree(to_pci_dev(dev));\n+}\n+\n  /**\n   * new_id_store - sysfs frontend to pci_add_dynid()\n   * @driver: target device driver\n@@ -214,11 +219,14 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,\n                 pdev->subsystem_vendor = subvendor;\n                 pdev->subsystem_device = subdevice;\n                 pdev->class = class;\n+               pdev->dev.release = _release_temp_device;\n\n+               /* Initialize the embedded struct device. */\n+               device_initialize(&pdev->dev);\n                 if (pci_match_device(pdrv, pdev))\n                         retval = -EEXIST;\n\n-               kfree(pdev);\n+               put_device(&pdev->dev);\n\n                 if (retval)\n                         return retval;\n>\n>>  \t\tif (pci_match_device(pdrv, pdev))\n>>  \t\t\tretval = -EEXIST;\n>>\n>>\n>> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731\n>> --\n>> 2.54.0.545.g6539524ca2-goog","headers":{"Return-Path":"\n <linux-pci+bounces-53249-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256\n header.s=20251104 header.b=bmrLkWIV;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-53249-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=google.com header.i=@google.com\n header.b=\"bmrLkWIV\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=209.85.214.180","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=google.com","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=google.com"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g4FpY54NPz1yHv\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 28 Apr 2026 06:41:25 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id BCA373084D8A\n\tfor <incoming@patchwork.ozlabs.org>; Mon, 27 Apr 2026 20:36:10 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 8A09D36E47F;\n\tMon, 27 Apr 2026 20:36:09 +0000 (UTC)","from mail-pl1-f180.google.com (mail-pl1-f180.google.com\n [209.85.214.180])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 37C3A366567\n\tfor <linux-pci@vger.kernel.org>; Mon, 27 Apr 2026 20:36:08 +0000 (UTC)","by mail-pl1-f180.google.com with SMTP id\n d9443c01a7336-2b46da8c48eso25205ad.1\n        for <linux-pci@vger.kernel.org>; Mon, 27 Apr 2026 13:36:08 -0700 (PDT)","from google.com (195.236.83.34.bc.googleusercontent.com.\n [34.83.236.195])\n        by smtp.gmail.com with ESMTPSA id\n d9443c01a7336-2b97ac8d439sm2960695ad.66.2026.04.27.13.36.06\n        (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n        Mon, 27 Apr 2026 13:36:06 -0700 (PDT)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777322169; cv=none;\n b=ohYEYhMyim1chT76LUKJ6w5AEJw0+rDkhLptxGo4fI5GPQhNODbQJpuhvT8qKxSn2n9cly0SoftK2xfcg1Z9nFBe/VrZWCzZJnF8xZ6KFlZdx4UAbqQDiugIyDWwv5+Gg45UzA3ebHRoEw9hCxkta2jr23mA26dx/ARuzsL4HRw=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777322169; c=relaxed/simple;\n\tbh=KeprCv7MW+Ce6GRs821xRLB6lzgXnDlvJi4UuP93Z0o=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=ARqahjsywEfwes7gaPCv21K2/c0CORKYBnAfjwaj0bFmF9pc6aSLztIlHmoCWgU1+NnR8FPCdZDcV9naJ80guEKq6vvQbIdydFsZvgZfm6W0QFLGOWjKAYVKQE41TCnoAUBEF+Xn3RO+ayoD1jOumbANb8h9yxBLg6bltTVUTYo=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=google.com;\n spf=pass smtp.mailfrom=google.com;\n dkim=pass (2048-bit key) header.d=google.com header.i=@google.com\n header.b=bmrLkWIV; arc=none smtp.client-ip=209.85.214.180","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=google.com; s=20251104; t=1777322167; x=1777926967;\n darn=vger.kernel.org;\n        h=in-reply-to:content-disposition:mime-version:references:message-id\n         :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to;\n        bh=5kLJiYyLuMrCMUKzFtT9+iIyaNr1wOEkpYjAcEB5YGU=;\n        b=bmrLkWIVSxk+L9G8lTDVFmbvafcTq8CDnHR2x/I4EEXR066PnMtIctudoC5XyxLYUj\n         4mjjxtGLo0z3xMlFfhtT5r4amgxpPB3XpiEOhgB1KdoLK56fltUxFIoVgHooAfrJbN/8\n         wskUCJ2Jl/TgbDaP+GWsjasyXxxj/HlhZOV1symkcEvzmjDtM+FMxP/mf8Sg9rbZpBoE\n         P1StnUXj8MHXW89CZZn5F2iybiffEdb9nPSwdpvIWemI1mUZnkoqITpzfGGwev7DnJTH\n         3ECRU5OfiNJs96/Kk0NeGtHtdBrlrkqUmTlSCWMgZ8Tf7De/mxEa5cI0f9QWkaAQwaJz\n         fqgw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n        d=1e100.net; s=20251104; t=1777322167; x=1777926967;\n        h=in-reply-to:content-disposition:mime-version:references:message-id\n         :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc\n         :subject:date:message-id:reply-to;\n        bh=5kLJiYyLuMrCMUKzFtT9+iIyaNr1wOEkpYjAcEB5YGU=;\n        b=U8NoelTXmmg9Vl5bxcIAJtYKAYNlFe44vzSGxsWYA5zX8Sh9UNAFLwGuiT/imM6BrC\n         R9mQFac+VH97KaQwvTVFUyfSls8W31AXxPJ1/rJqqaEAIeOvb7BEurdx0hmU/8rmt0aP\n         zihApWfpGsfYToKrlOHPomRR+e0ioegtpPqHGXtqUVDT+NODMJZhsKYV5mvXVNiTThAN\n         5OYVSyfz1Pg7a89awjA8Tp3x4phGtt3tqLwzIRePuKD1v6poF1Ri5Ktc6pLnCN/4Jen0\n         HLuF2Zyh7uMp2l99qD1D4wMgm6f0lY3uBWyVkBgQutKUBgpWHBhryqd/QeTXOCDSNOKB\n         bwHg==","X-Forwarded-Encrypted":"i=1;\n AFNElJ+ubC8SUZvUa7G1t5BKt/M1hr8Fqp4UPVvHRy0SJnT7LJyRGudxWbbhjBGcN1qy21La7ifPpxDI6Eg=@vger.kernel.org","X-Gm-Message-State":"AOJu0Yz6aQonR/X3MeY4VQ2LQkrJWvazhiQOxojXTJ3pRGXUZbdqRFgw\n\tgSOwj9c+9g0+udbH6b8R8SBpKdgodyZONDBxBTfMvu+wZkCC636foerL+NnqPSN3NS2lNmMT2hI\n\t8CAk64g==","X-Gm-Gg":"AeBDiesi/c7RQ/vzHjvEECB6tPtuv55d5ujKI6EJNgLyDdjGluGP/mtTZ1usE5qSpNj\n\tBSA4kqOSQuIwrjHGOkmO5Fn/288nQSD8X2+aCKIIn2aWkQuiBY8MKCYf1u15dxvG0MiKAfvI3G0\n\tI5Nld80wAZJuIdYyeU7b2M5lbSYPopEoBGmEMmarIrPLFdXuiiqerJ04nBEvxF5v6s5vcT4aAC0\n\tNAmsHAD8oHVWLjXredKR0mfvnpVwSL75rzRoh98S8X9bWqfvqepR2y2rH7LVa38Og+8lD/BDIPI\n\tBk4BUIDUcHDPPAyMwq+QPbeTIUzPDH+X0af0wg3f9XG9MmLDXA0sH2Y/3IieyzAlvCF9WQ9/vRp\n\tNdFj7cFtIpqq4tETH4t72gtKGkixMxYLKaw+Iibb5Ii7lM2s+KRXDze+4UsvU3dXd2kC+ShM1cR\n\tufHlDufzlTML2fT9fDEuGHMXy1BrWf7qgk2XC/B4XSmZphldsldK9WvKopmUJ3TIh6AoB6VOox3\n\tJBhraYNQqA=","X-Received":"by 2002:a17:902:c40e:b0:2b0:b925:da98 with SMTP id\n d9443c01a7336-2b97c25051bmr274785ad.19.1777322167052;\n        Mon, 27 Apr 2026 13:36:07 -0700 (PDT)","Date":"Mon, 27 Apr 2026 20:36:02 +0000","From":"Samiullah Khawaja <skhawaja@google.com>","To":"Danilo Krummrich <dakr@kernel.org>","Cc":"Bjorn Helgaas <bhelgaas@google.com>,\n\tGui-Dong Han <hanguidong02@gmail.com>,\n Greg Kroah-Hartman <gregkh@linuxfoundation.org>,\n\tAlex Williamson <alex@shazbot.org>,\n \"open list:PCI SUBSYSTEM\" <linux-pci@vger.kernel.org>,\n\topen list <linux-kernel@vger.kernel.org>, dmatlack@google.com","Subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","Message-ID":"<ae-_EmuuoQTqD6wg@google.com>","References":"<20260427193139.2109938-1-skhawaja@google.com>\n <DI472FJ8UOG4.1JGTQK7W1RAX@kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"<linux-pci.vger.kernel.org>","List-Subscribe":"<mailto:linux-pci+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pci+unsubscribe@vger.kernel.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii; format=flowed","Content-Disposition":"inline","In-Reply-To":"<DI472FJ8UOG4.1JGTQK7W1RAX@kernel.org>"}},{"id":3683005,"web_url":"http://patchwork.ozlabs.org/comment/3683005/","msgid":"<DI48VQVY1LJN.3PKEQB6KX0S3J@kernel.org>","list_archive_url":null,"date":"2026-04-27T21:11:38","subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","submitter":{"id":89037,"url":"http://patchwork.ozlabs.org/api/people/89037/","name":"Danilo Krummrich","email":"dakr@kernel.org"},"content":"On Mon Apr 27, 2026 at 10:36 PM CEST, Samiullah Khawaja wrote:\n> On Mon, Apr 27, 2026 at 09:46:20PM +0200, Danilo Krummrich wrote:\n>>On Mon Apr 27, 2026 at 9:31 PM CEST, Samiullah Khawaja wrote:\n>>> Fixes: cb3d1049f4ea (\"driver core: generalize driver_override in struct device\")\n>>\n>>I don't think anything is wrong with this commit, and it seems unrelated.\n>\n> I will remove this one.\n>>\n>>> Fixes: 10a4206a2401 (\"PCI: use generic driver_override infrastructure\")\n>>\n>>I'm also not sure that this one contains the root cause, despite revealing the\n>>actual issue, more below.\n>\n> I see your point. I understand the actual issue is that the temporary\n> pci_dev struct was not init and that was added in 2014. I added this\n> since this is where the regression happened.\n\nYeah, I did not mean to say that this tag is wrong -- it is where the regression\nhappened. But it may be worth adding both, this one and the one that did omit\ndevice_initialize() in the first place.\n\nMore in general, this can matter because it influences how (far) it is\nbackported in stable trees.\n\nWhile rather unlikely in this case, there could be a subsequent fix that is\nincompatible with device_initialize() not being called that may be backported\nevent further.\n\nThis would not regress in recent trees as it would have been fixed there\nalready, but might regress in an ancient stable tree, that did not get this fix.\n\n>>> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>\n>>> ---\n>>>  drivers/pci/pci-driver.c | 5 +++++\n>>>  1 file changed, 5 insertions(+)\n>>>\n>>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c\n>>> index d10ece0889f0..5f453213b8c5 100644\n>>> --- a/drivers/pci/pci-driver.c\n>>> +++ b/drivers/pci/pci-driver.c\n>>> @@ -215,6 +215,11 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,\n>>>  \t\tpdev->subsystem_device = subdevice;\n>>>  \t\tpdev->class = class;\n>>>\n>>> +\t\t/*\n>>> +\t\t * Initialize the embedded struct device driver_override lock to\n>>> +\t\t * avoid the lockdep errors.\n>>> +\t\t */\n>>> +\t\tspin_lock_init(&pdev->dev.driver_override.lock);\n>>\n>>Can't we just call device_initialize() and set pdev->dev.release to a new\n>>function that just calls kfree()?\n>>\n>>This way nothing of that kind can ever happen again; it is hard to predict that\n>>a device is used without ever being initialized.\n>\n> Agreed.\n>\n> yes we can set the release function and call device_initialize().\n>\n> Should I send v2 with something like following:\n>\n> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c\n> index d10ece0889f0..634b4311bf5d 100644\n> --- a/drivers/pci/pci-driver.c\n> +++ b/drivers/pci/pci-driver.c\n> @@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,\n>          return NULL;\n>   }\n>\n> +static void _release_temp_device(struct device *dev)\n> +{\n> +       kfree(to_pci_dev(dev));\n> +}\n> +\n>   /**\n>    * new_id_store - sysfs frontend to pci_add_dynid()\n>    * @driver: target device driver\n> @@ -214,11 +219,14 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,\n>                  pdev->subsystem_vendor = subvendor;\n>                  pdev->subsystem_device = subdevice;\n>                  pdev->class = class;\n> +               pdev->dev.release = _release_temp_device;\n>\n> +               /* Initialize the embedded struct device. */\n> +               device_initialize(&pdev->dev);\n>                  if (pci_match_device(pdrv, pdev))\n>                          retval = -EEXIST;\n>\n> -               kfree(pdev);\n> +               put_device(&pdev->dev);\n\nOverall Looks good. Not sure about the _release_temp_device() name and I think\nthe comment above device_initialize() may be unnecessary, but I'm pretty sure\nthe PCI maintainers will have an opinion on that. :)","headers":{"Return-Path":"\n <linux-pci+bounces-53265-incoming=patchwork.ozlabs.org@vger.kernel.org>","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=EbktTdj3;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.105.105.114; helo=tor.lore.kernel.org;\n envelope-from=linux-pci+bounces-53265-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"EbktTdj3\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org [172.105.105.114])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g4GTY0kXqz1xrS\n\tfor <incoming@patchwork.ozlabs.org>; Tue, 28 Apr 2026 07:11:45 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 2D6393014774\n\tfor <incoming@patchwork.ozlabs.org>; Mon, 27 Apr 2026 21:11:43 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 172D43AB26B;\n\tMon, 27 Apr 2026 21:11:42 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id E6C1A3AA517;\n\tMon, 27 Apr 2026 21:11:41 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 15F46C19425;\n\tMon, 27 Apr 2026 21:11:39 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777324302; cv=none;\n b=UkRgIth2Q+ckQZCU/s7M2JRVALlD2N3RchGxt1oyBOQ5u0p75kUlXceLafs7vbsIgpOPnxzpWUYVsD6u01ZEa+cgTxyZ1st76eZrXYebNxux8q/6D/wK+/5Ivj+BRrdjqXsrJBWEhAxuPMig1QfHd642oQ8IdtvMjy6gcQCM7JA=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777324302; c=relaxed/simple;\n\tbh=eCpFyIQwjTL0FLnJ9C988NSyGD1GctZLUSJP2KJCfYw=;\n\th=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From:\n\t References:In-Reply-To;\n b=Wa9YtqT7Mm8zrg6p/8tyfB39yuEEuzWfVakh1rRIq3LIrQ8U4EAuWJ6oIwokDn+obFnEjUXgOGGipT7oHMQo9ZcOyI649r4Ls/Y1vxWfRgd6IQ6sYks4DmQ6IVZoW7909i1imHwcseoxwVYBCIyaenOu1tGw5CYex8FWvnhxH54=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=EbktTdj3; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1777324301;\n\tbh=eCpFyIQwjTL0FLnJ9C988NSyGD1GctZLUSJP2KJCfYw=;\n\th=Date:Subject:Cc:To:From:References:In-Reply-To:From;\n\tb=EbktTdj3IgWyZPwRQYCkuNqeWZ8NhoHT4Y1GB1m6nNjaHWbkcZHg8PHEEFlzuiQHD\n\t 69S5nRps+E3F1e3GxY+aOEQ8Be9iw/PYFHHHE+mCLi6i7wT0myhDrM3gvF9CErRFYO\n\t 52JAATsEg8TXuAuvViQJCroYOS2HEsea5scR6Hs7FVjWLztCIIY8HLy775FdGHXxaZ\n\t 2UDod8oYI2jVV6+vEyUXVAJE6nqOCBfa7tHO0xQuX4Ze2WrZk39KCgYAsHnzKpG1w0\n\t XDGq/9iyeAYoqW2teG8iOHqLgGrtjie/JHaOnhMCO3S37dGHcx51B9DVYIgN4oKKJm\n\t I5rWtjgDYWxbA==","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"<linux-pci.vger.kernel.org>","List-Subscribe":"<mailto:linux-pci+subscribe@vger.kernel.org>","List-Unsubscribe":"<mailto:linux-pci+unsubscribe@vger.kernel.org>","Mime-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=UTF-8","Date":"Mon, 27 Apr 2026 23:11:38 +0200","Message-Id":"<DI48VQVY1LJN.3PKEQB6KX0S3J@kernel.org>","Subject":"Re: [PATCH] PCI: Init driver override spinlock in new_id_store()","Cc":"\"Bjorn Helgaas\" <bhelgaas@google.com>, \"Gui-Dong Han\"\n <hanguidong02@gmail.com>, \"Greg Kroah-Hartman\"\n <gregkh@linuxfoundation.org>, \"Alex Williamson\" <alex@shazbot.org>, \"open\n list:PCI SUBSYSTEM\" <linux-pci@vger.kernel.org>, \"open list\"\n <linux-kernel@vger.kernel.org>, <dmatlack@google.com>","To":"\"Samiullah Khawaja\" <skhawaja@google.com>","From":"\"Danilo Krummrich\" <dakr@kernel.org>","References":"<20260427193139.2109938-1-skhawaja@google.com>\n <DI472FJ8UOG4.1JGTQK7W1RAX@kernel.org> <ae-_EmuuoQTqD6wg@google.com>","In-Reply-To":"<ae-_EmuuoQTqD6wg@google.com>"}}]