Re: [PATCH 01/16] PCI/P2PDMA: Pass gfp_mask flags to upstream_bridge_distance_warn()

From: Don Dutile
Date: Tue May 11 2021 - 12:23:42 EST


On 5/11/21 12:12 PM, Logan Gunthorpe wrote:

On 2021-05-11 10:05 a.m., Don Dutile wrote:
On 5/1/21 11:58 PM, John Hubbard wrote:
On 4/8/21 10:01 AM, Logan Gunthorpe wrote:
In order to call upstream_bridge_distance_warn() from a dma_map function,
it must not sleep. The only reason it does sleep is to allocate the seqbuf
to print which devices are within the ACS path.

Switch the kmalloc call to use a passed in gfp_mask and don't print that
message if the buffer fails to be allocated.

Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
---
  drivers/pci/p2pdma.c | 21 +++++++++++----------
  1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 196382630363..bd89437faf06 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -267,7 +267,7 @@ static int pci_bridge_has_acs_redir(struct pci_dev *pdev)
    static void seq_buf_print_bus_devfn(struct seq_buf *buf, struct pci_dev *pdev)
  {
-    if (!buf)
+    if (!buf || !buf->buffer)
This is not great, sort of from an overall design point of view, even though
it makes the rest of the patch work. See below for other ideas, that will
avoid the need for this sort of odd point fix.

+1.
In fact, I didn't see how the kmalloc was changed... you refactored the code to pass-in the
GFP_KERNEL that was originally hard-coded into upstream_bridge_distance_warn();
I don't see how that avoided the kmalloc() call.
in fact, I also see you lost a failed kmalloc() check, so it seems to have taken a step back.
I've changed this in v2 to just use some memory allocated on the stack.
Avoids this argument all together.

Logan

Looking fwd to the v2; again, my apologies for the delay, and the redundancy it's adding to your feedback review & changes.
-Don