-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Monday, July 31, 2023 8:31 PM
To: Haiyang Zhang <haiyangz@microsoft.com>
Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Dexuan Cui
[off-list ref]; KY Srinivasan [off-list ref]; Paul Rosswurm
[off-list ref]; olaf@aepfle.de; vkuznets@redhat.com;
davem@davemloft.net; wei.liu@kernel.org; edumazet@google.com;
pabeni@redhat.com; leon@kernel.org; Long Li [off-list ref];
ssengar@linux.microsoft.com; linux-rdma@vger.kernel.org;
daniel@iogearbox.net; john.fastabend@gmail.com; bpf@vger.kernel.org;
ast@kernel.org; Ajay Sharma [off-list ref]; hawk@kernel.org;
tglx@linutronix.de; shradhagupta@linux.microsoft.com; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH V4,net-next] net: mana: Add page pool for RX buffers

On Fri, 28 Jul 2023 14:46:07 -0700 Haiyang Zhang wrote:

quoted

 static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
-                        dma_addr_t *da, bool is_napi)
+                        dma_addr_t *da, bool *from_pool, bool is_napi)
 {
    struct page *page;
    void *va;

+   *from_pool = false;
+
    /* Reuse XDP dropped page if available */
    if (rxq->xdp_save_va) {
            va = rxq->xdp_save_va;

@@ -1533,17 +1543,22 @@ static void *mana_get_rxfrag(struct mana_rxq

*rxq, struct device *dev,

quoted

                    return NULL;
            }
    } else {
-           page = dev_alloc_page();
+           page = page_pool_dev_alloc_pages(rxq->page_pool);
            if (!page)
                    return NULL;

+           *from_pool = true;
            va = page_to_virt(page);
    }

    *da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
                         DMA_FROM_DEVICE);
    if (dma_mapping_error(dev, *da)) {
-           put_page(virt_to_head_page(va));
+           if (*from_pool)
+                   page_pool_put_full_page(rxq->page_pool, page,

is_napi);

AFAICT you only pass the is_napi to recycle in case of error?
It's fine to always pass in false, passing true enables some
optimizations but it's not worth trying to optimize error paths.

Yes, this place is only for the error path. I will pass in false.

Otherwise you may be passing in true, even tho budget was 0,
see the recently added warnings in this doc:

https://www.ker/
nel.org%2Fdoc%2Fhtml%2Fnext%2Fnetworking%2Fnapi.html&data=05%7C01%7
Chaiyangz%40microsoft.com%7C82fcd2d20fe54dd8cd4808db9226a2c7%7C72f9
88bf86f141af91ab2d7cd011db47%7C1%7C0%7C638264466881746523%7CUnkn
own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
WwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=D9ac4TnYOSPwGmk%2FKX
G4buu%2FKT7J%2BuH8lAJNPl%2FRWy4%3D&reserved=0

In general the driver seems to be processing Rx regardless
of budget? This looks like a bug which should be fixed with
a separate patch for the net tree..

Thanks, I will look into and fix this in a separate patch.

- Haiyang