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

Bjorn Helgaas
2021-03-12T20:39:41Z

On Thu, Mar 11, 2021 at 04:31:31PM -0700, Logan Gunthorpe wrote:
> In order to call this function from a dma_map function, it must not sleep.
> The only reason it does sleep so to allocate the seqbuf to print
> which devices are within the ACS path.

s/this function/upstream_bridge_distance_warn()/ ?
s/so to/is to/

Maybe the subject could say something about the purpose, e.g., allow
calling from atomic context or something? "Pass gfp_mask flags" sort
of restates what we can read from the patch, but without the
motivation of why this is useful.

> 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@deltatee.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> 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)
> return;
>
> seq_buf_printf(buf, "%s;", pci_name(pdev));
> @@ -495,25 +495,26 @@ upstream_bridge_distance(struct pci_dev *provider, struct pci_dev *client,
>
> static enum pci_p2pdma_map_type
> upstream_bridge_distance_warn(struct pci_dev *provider, struct pci_dev *client,
> - int *dist)
> + int *dist, gfp_t gfp_mask)
> {
> struct seq_buf acs_list;
> bool acs_redirects;
> int ret;
>
> - seq_buf_init(&acs_list, kmalloc(PAGE_SIZE, GFP_KERNEL), PAGE_SIZE);
> - if (!acs_list.buffer)
> - return -ENOMEM;
> + seq_buf_init(&acs_list, kmalloc(PAGE_SIZE, gfp_mask), PAGE_SIZE);
>
> ret = upstream_bridge_distance(provider, client, dist, &acs_redirects,
> &acs_list);
> if (acs_redirects) {
> pci_warn(client, "ACS redirect is set between the client and provider (%s)\n",
> pci_name(provider));
> - /* Drop final semicolon */
> - acs_list.buffer[acs_list.len-1] = 0;
> - pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
> - acs_list.buffer);
> +
> + if (acs_list.buffer) {
> + /* Drop final semicolon */
> + acs_list.buffer[acs_list.len - 1] = 0;
> + pci_warn(client, "to disable ACS redirect for this path, add the kernel parameter: pci=disable_acs_redir=%s\n",
> + acs_list.buffer);
> + }
> }
>
> if (ret == PCI_P2PDMA_MAP_NOT_SUPPORTED) {
> @@ -566,7 +567,7 @@ int pci_p2pdma_distance_many(struct pci_dev *provider, struct device **clients,
>
> if (verbose)
> ret = upstream_bridge_distance_warn(provider,
> - pci_client, &distance);
> + pci_client, &distance, GFP_KERNEL);
> else
> ret = upstream_bridge_distance(provider, pci_client,
> &distance, NULL, NULL);
> --
> 2.20.1