Re: [PATCH v4 18/18] PCI/P2PDMA: Log detailed ACS routing diagnostics

On Fri, Aug 21, 2026 at 08:02:45PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unconditional debug logging in the high-frequency P2PDMA mapping fast path causes severe log flooding.
> --
> 
> --- Patch [18]: [PATCH v4 18/18] PCI/P2PDMA: Log detailed ACS routing diagnostics ---
> commit 6ad59be5d5d93d88cf70cbce85e9046d3c5b0adb
> Author: Leon Romanovsky <[email protected]>
> 
>     PCI/P2PDMA: Log detailed ACS routing diagnostics
> 
> This commit adds detailed ACS routing diagnostics to assist in identifying
> P2PDMA mapping failures. It emits info-level logs covering path details,
> ACS controls, and cache state.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index e54901d21d8a2..b79eb4ce09f47 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> 
> [ ... ]
> 
> > @@ -1237,23 +1467,38 @@ enum pci_p2pdma_map_type pci_p2pdma_map_type(struct p2pdma_provider *provider,
> >  	struct pci_dev *pdev = to_pci_dev(provider->owner);
> >  	struct pci_dev *client;
> >  	struct pci_p2pdma *p2pdma;
> > +	unsigned long cache_index;
> > +	bool provider_state;
> >  	int dist;
> >  
> > -	if (!pdev->p2pdma)
> > +	if (!pdev->p2pdma) {
> > +		pci_info(pdev,
> > +			 "P2PDMA ACS debug: map lookup rejected; provider state is absent\n");
> >  		return PCI_P2PDMA_MAP_NOT_SUPPORTED;
> > +	}
> >  
> > -	if (!dev_is_pci(dev))
> > +	if (!dev_is_pci(dev)) {
> > +		dev_info(dev,
> > +			 "P2PDMA ACS debug: provider=%s map lookup rejected; client is not PCI\n",
> > +			 pci_name(pdev));
> >  		return PCI_P2PDMA_MAP_NOT_SUPPORTED;
> > +	}
> >  
> >  	client = to_pci_dev(dev);
> > +	cache_index = map_types_idx(client);
> >  
> >  	rcu_read_lock();
> >  	p2pdma = rcu_dereference(pdev->p2pdma);
> >  
> >  	if (p2pdma)
> >  		type = xa_to_value(xa_load(&p2pdma->map_types,
> > -					   map_types_idx(client)));
> > +					   cache_index));
> > +	provider_state = !!p2pdma;
> >  	rcu_read_unlock();
> > +	pci_info(client,
> > +		 "P2PDMA ACS debug: map lookup provider=%s index=%#lx cached=%s(%d) provider-state=%u\n",
> > +		 pci_name(pdev), cache_index, pci_p2pdma_map_type_name(type),
> > +		 type, provider_state);
> 
> [Severity: High]
> Does this code risk flooding the kernel log buffer and causing severe performance
> degradation?
> 
> Since pci_p2pdma_map_type() is invoked on the DMA mapping fast path (for
> example, once per P2PDMA I/O request via pci_p2pdma_state() in dma_map_sg()
> loops), this unconditional logging will execute on every single P2PDMA
> map lookup even when a cached route is successfully used.
> 
> Will this saturate the kernel log buffer and cause severe printk/console lock
> contention for any P2PDMA-capable block I/O or RDMA operation?

No, these are debug prints and are not enabled by default. ACS
successes and failures are deterministic, and a single operation is
sufficient to determine the cause of a failure.

Thanks

> 
> >  
> >  	if (type == PCI_P2PDMA_MAP_UNKNOWN)
> >  		return calc_map_type_and_dist(pdev, client, &dist, true);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18
>