From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 37BAA1D5170 for ; Mon, 27 Apr 2026 20:36:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777322170; cv=none; b=MRNACj4YFMDpGpcW1vdTLbeUZxmDB1mF4+o+tJdQIk24HLMTof7z9104lELZ8QJDXrhPhiJ/vl+bKSgHbI0LaR0XuPU7s/gIZPbdq4US51m/BLqUd0r5zgDIqD1IkthBuYbaK4lI8zeci7j/evH+q0rUzDZE1vab/uHignZwxAE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777322170; c=relaxed/simple; bh=KeprCv7MW+Ce6GRs821xRLB6lzgXnDlvJi4UuP93Z0o=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ro9C6R3bcxzlJDhHmmOTMvEiXO98EYBrHgMsKBxBs5tyeIBYo1z8dCYBvNLlWAlDcuHXPSQ/PffZeC8zpEUQL2DP+ZbbpMOvXAWIzunCOtB0Jyjir/ahk8ZaMnsxBAyIHwigAKYcxMhVqMn8zI5Mmm/e7CRYRTs/ZlqpQdC8Flg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=bmrLkWIV; arc=none smtp.client-ip=209.85.214.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="bmrLkWIV" Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-2b2591757fbso28395ad.0 for ; Mon, 27 Apr 2026 13:36:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1777322167; x=1777926967; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=5kLJiYyLuMrCMUKzFtT9+iIyaNr1wOEkpYjAcEB5YGU=; b=bmrLkWIVSxk+L9G8lTDVFmbvafcTq8CDnHR2x/I4EEXR066PnMtIctudoC5XyxLYUj 4mjjxtGLo0z3xMlFfhtT5r4amgxpPB3XpiEOhgB1KdoLK56fltUxFIoVgHooAfrJbN/8 wskUCJ2Jl/TgbDaP+GWsjasyXxxj/HlhZOV1symkcEvzmjDtM+FMxP/mf8Sg9rbZpBoE P1StnUXj8MHXW89CZZn5F2iybiffEdb9nPSwdpvIWemI1mUZnkoqITpzfGGwev7DnJTH 3ECRU5OfiNJs96/Kk0NeGtHtdBrlrkqUmTlSCWMgZ8Tf7De/mxEa5cI0f9QWkaAQwaJz fqgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777322167; x=1777926967; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5kLJiYyLuMrCMUKzFtT9+iIyaNr1wOEkpYjAcEB5YGU=; b=AKp1TCecOoQLbE8zzafQNNl5AXAw1Du6m36z1fK44W/tsxk5rNxK4g0eRfH4PgFzY+ 2n9cdCwy+QnPZ+At1dtBR1qBtN/c/84pTgoNNn3i4hwmfUCHYmZ0EldTtRC7UgXKuR6v 2vPLdOAUDZSPk6fwKNBDx6GUPKWyj/2/rSU/B4X2pIT6VKLnCdMPiNClnIeijOQI79XI Z07bdh75MMUYFDZpT5a9oZGxkUY8+rG2i88szLjnhWViFgGTiGRLoL3o4+kof/0VuXX1 jcxwZse0dU2m+hX2Bifyk5H/dP3+3ucX7lKlN6Mu1n0FPHd7Oc8h5hQpyWNl9QNrQ+pg rEWA== X-Forwarded-Encrypted: i=1; AFNElJ8FOZ5MszuZcxa9BgiVdiOawDRrpF2Yxt8obB0xHbryOwyU7j4KeJ5X0bTcp3dDPj8xja3chS4O2crT8m0=@vger.kernel.org X-Gm-Message-State: AOJu0YxlU2O0S+nxS5RNtrYQn0pFD6C5qMMzSncZ4S+X0kmZgIhGSXua qY3TtvMmxDBwm3iXN5llJ6KYAbiYDEA4vdGdpp5GXUpkK8yd8NPSvWF7HixjXPyrIg== X-Gm-Gg: AeBDiesiLZZVQzjedfbKa5lx/JRRTYnmmm3W4zi3IJBtSuP/mdsTWa9qanE6vTzzIYa P36Yo6MN6Yc2NjErblazA0wz2eTu5YfBjvml6YGOVnDsu5RYA6CisGrVSMabX1VD97WE1mmZyeg PjhQVGRm3yvtL/XjK+YyKJEqdaINmhHjBAotTrWLDM+F1HNm9X27NIAXwifJY1+F7WGLjYSUlGw WXKjf/PTyGKvcvuK2bjKfdP0OQ6d9bWm7UwFfBgi9Udj4TZwwtCugqO6tm0h+B0bf39X9Ex2zM/ BZKD+Lwq4DzYVww5ucMGv4Wo945pXihv31DXmF67BtB/P02FxEQJ4b1wSWHMH8yH3xnIO8B5MV1 ZlwZNfM0YNuRhr73GPaEHXSxI/X4KZLzLwU5Tg2+nZda16QY9i08XVLrDjv0YsWunr9cQoAD3QS KEjo4gFLPu6ESxGvkq/addjl10JKgkXUgkWWorcs8RgJ4NPlr3YqpNISfj0MLZjarHazmg3PC8n sPgG56Tigw= X-Received: by 2002:a17:902:c40e:b0:2b0:b925:da98 with SMTP id d9443c01a7336-2b97c25051bmr274785ad.19.1777322167052; Mon, 27 Apr 2026 13:36:07 -0700 (PDT) Received: from google.com (195.236.83.34.bc.googleusercontent.com. [34.83.236.195]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2b97ac8d439sm2960695ad.66.2026.04.27.13.36.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Apr 2026 13:36:06 -0700 (PDT) Date: Mon, 27 Apr 2026 20:36:02 +0000 From: Samiullah Khawaja To: Danilo Krummrich Cc: Bjorn Helgaas , Gui-Dong Han , Greg Kroah-Hartman , Alex Williamson , "open list:PCI SUBSYSTEM" , open list , dmatlack@google.com Subject: Re: [PATCH] PCI: Init driver override spinlock in new_id_store() Message-ID: References: <20260427193139.2109938-1-skhawaja@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: On Mon, Apr 27, 2026 at 09:46:20PM +0200, Danilo Krummrich wrote: >On Mon Apr 27, 2026 at 9:31 PM CEST, Samiullah Khawaja wrote: >> Fixes: cb3d1049f4ea ("driver core: generalize driver_override in struct device") > >I don't think anything is wrong with this commit, and it seems unrelated. I will remove this one. > >> Fixes: 10a4206a2401 ("PCI: use generic driver_override infrastructure") > >I'm also not sure that this one contains the root cause, despite revealing the >actual issue, more below. I see your point. I understand the actual issue is that the temporary pci_dev struct was not init and that was added in 2014. I added this since this is where the regression happened. > >> Signed-off-by: Samiullah Khawaja >> --- >> drivers/pci/pci-driver.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> index d10ece0889f0..5f453213b8c5 100644 >> --- a/drivers/pci/pci-driver.c >> +++ b/drivers/pci/pci-driver.c >> @@ -215,6 +215,11 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf, >> pdev->subsystem_device = subdevice; >> pdev->class = class; >> >> + /* >> + * Initialize the embedded struct device driver_override lock to >> + * avoid the lockdep errors. >> + */ >> + spin_lock_init(&pdev->dev.driver_override.lock); > >Can't we just call device_initialize() and set pdev->dev.release to a new >function that just calls kfree()? > >This way nothing of that kind can ever happen again; it is hard to predict that >a device is used without ever being initialized. Agreed. yes we can set the release function and call device_initialize(). Should I send v2 with something like following: diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index d10ece0889f0..634b4311bf5d 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv, return NULL; } +static void _release_temp_device(struct device *dev) +{ + kfree(to_pci_dev(dev)); +} + /** * new_id_store - sysfs frontend to pci_add_dynid() * @driver: target device driver @@ -214,11 +219,14 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf, pdev->subsystem_vendor = subvendor; pdev->subsystem_device = subdevice; pdev->class = class; + pdev->dev.release = _release_temp_device; + /* Initialize the embedded struct device. */ + device_initialize(&pdev->dev); if (pci_match_device(pdrv, pdev)) retval = -EEXIST; - kfree(pdev); + put_device(&pdev->dev); if (retval) return retval; > >> if (pci_match_device(pdrv, pdev)) >> retval = -EEXIST; >> >> >> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731 >> -- >> 2.54.0.545.g6539524ca2-goog