From: Bjorn Helgaas <helgaas@kernel.org>
To: Imre Deak <imre.deak@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
Dan Williams <dan.j.williams@intel.com>,
Jani Saarinen <jani.saarinen@intel.com>,
Dave Jiang <dave.jiang@intel.com>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org
Subject: Re: [core-for-CI PATCH] PCI: Make PCI cfg_access_lock lockdep key a singleton
Date: Thu, 30 May 2024 11:46:26 -0500 [thread overview]
Message-ID: <20240530164626.GA550564@bhelgaas> (raw)
In-Reply-To: <20240529114901.344655-1-imre.deak@intel.com>
[+cc linux-pci, this is a follow-up to 7e89efc6e9e4, which appeared in
v6.10-rc1 via the PCI tree]
On Wed, May 29, 2024 at 02:49:01PM +0300, Imre Deak wrote:
> From: Dan Williams <dan.j.williams@intel.com>
>
> The new lockdep annotation for cfg_access_lock naively registered a new
> key per device. This is overkill and leads to warnings on hash
> collisions at dynamic registration time:
>
> WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:1226 lockdep_register_key+0xb0/0x240
> RIP: 0010:lockdep_register_key+0xb0/0x240
> [..]
> Call Trace:
> <TASK>
> ? __warn+0x8c/0x190
> ? lockdep_register_key+0xb0/0x240
> ? report_bug+0x1f8/0x200
> ? handle_bug+0x3c/0x70
> ? exc_invalid_op+0x18/0x70
> ? asm_exc_invalid_op+0x1a/0x20
> ? lockdep_register_key+0xb0/0x240
> pci_device_add+0x14b/0x560
> ? pci_setup_device+0x42e/0x6a0
> pci_scan_single_device+0xa7/0xd0
> p2sb_scan_and_cache_devfn+0xc/0x90
> p2sb_fs_init+0x15f/0x170
>
> Switch to a shared static key for all instances.
>
> Fixes: 7e89efc6e9e4 ("PCI: Lock upstream bridge for pci_reset_function()")
> Reported-by: Jani Saarinen <jani.saarinen@intel.com>
> Closes: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_14834/bat-apl-1/boot0.txt
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/pci/probe.c | 7 ++++---
> include/linux/pci.h | 1 -
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8e696e547565c..15168881ec941 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2533,6 +2533,8 @@ static void pci_set_msi_domain(struct pci_dev *dev)
> dev_set_msi_domain(&dev->dev, d);
> }
>
> +static struct lock_class_key cfg_access_key;
> +
> void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> {
> int ret;
> @@ -2546,9 +2548,8 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> dev->dev.dma_mask = &dev->dma_mask;
> dev->dev.dma_parms = &dev->dma_parms;
> dev->dev.coherent_dma_mask = 0xffffffffull;
> - lockdep_register_key(&dev->cfg_access_key);
> - lockdep_init_map(&dev->cfg_access_lock, dev_name(&dev->dev),
> - &dev->cfg_access_key, 0);
> + lockdep_init_map(&dev->cfg_access_lock, "&dev->cfg_access_lock",
> + &cfg_access_key, 0);
>
> dma_set_max_seg_size(&dev->dev, 65536);
> dma_set_seg_boundary(&dev->dev, 0xffffffff);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index fb004fd4e8890..5bece7fd11f88 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -413,7 +413,6 @@ struct pci_dev {
> struct resource driver_exclusive_resource; /* driver exclusive resource ranges */
>
> bool match_driver; /* Skip attaching driver */
> - struct lock_class_key cfg_access_key;
> struct lockdep_map cfg_access_lock;
>
> unsigned int transparent:1; /* Subtractive decode bridge */
> --
> 2.43.3
>
next prev parent reply other threads:[~2024-05-30 16:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-29 11:49 [core-for-CI PATCH] PCI: Make PCI cfg_access_lock lockdep key a singleton Imre Deak
2024-05-29 13:25 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
2024-05-29 13:33 ` ✓ Fi.CI.BAT: success " Patchwork
2024-05-29 15:33 ` [core-for-CI PATCH] " Saarinen, Jani
2024-05-29 15:37 ` Jani Nikula
2024-05-30 13:03 ` ✗ Fi.CI.IGT: failure for " Patchwork
2024-05-30 16:46 ` Bjorn Helgaas [this message]
2024-05-30 17:39 ` [core-for-CI PATCH] " Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240530164626.GA550564@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=imre.deak@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.saarinen@intel.com \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.