[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89i+6HOE8rs5S3OemjrY9yKGkUb0ZMcs3+npGCLNSFROoUg@mail.gmail.com>
Date: Wed, 22 Nov 2023 14:53:00 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, pabeni@...hat.com,
almasrymina@...gle.com, hawk@...nel.org, ilias.apalodimas@...aro.org,
dsahern@...il.com, dtatulea@...dia.com, willemb@...gle.com
Subject: Re: [PATCH net-next v3 12/13] net: page_pool: mute the periodic
warning for visible page pools
On Wed, Nov 22, 2023 at 4:44 AM Jakub Kicinski <kuba@...nel.org> wrote:
>
> Mute the periodic "stalled pool shutdown" warning if the page pool
> is visible to user space. Rolling out a driver using page pools
> to just a few hundred hosts at Meta surfaces applications which
> fail to reap their broken sockets. Obviously it's best if the
> applications are fixed, but we don't generally print warnings
> for application resource leaks. Admins can now depend on the
> netlink interface for getting page pool info to detect buggy
> apps.
>
> While at it throw in the ID of the pool into the message,
> in rare cases (pools from destroyed netns) this will make
> finding the pool with a debugger easier.
>
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> net/core/page_pool.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 3d0938a60646..c2e7c9a6efbe 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -897,18 +897,21 @@ static void page_pool_release_retry(struct work_struct *wq)
> {
> struct delayed_work *dwq = to_delayed_work(wq);
> struct page_pool *pool = container_of(dwq, typeof(*pool), release_dw);
> + void *netdev;
> int inflight;
>
> inflight = page_pool_release(pool);
> if (!inflight)
> return;
>
> - /* Periodic warning */
> - if (time_after_eq(jiffies, pool->defer_warn)) {
> + /* Periodic warning for page pools the user can't see */
> + netdev = READ_ONCE(pool->slow.netdev);
> + if (time_after_eq(jiffies, pool->defer_warn) &&
> + (!netdev || netdev == NET_PTR_POISON)) {
> int sec = (s32)((u32)jiffies - (u32)pool->defer_start) / HZ;
Orthogonal to your patch, but this probably could avoid all these casts.
long sec = (jiffies - pool->defer_start) / HZ;
>
> - pr_warn("%s() stalled pool shutdown %d inflight %d sec\n",
> - __func__, inflight, sec);
> + pr_warn("%s() stalled pool shutdown: id %u, %d inflight %d sec\n",
> + __func__, pool->user.id, inflight, sec);
> pool->defer_warn = jiffies + DEFER_WARN_INTERVAL;
> }
>
Reviewed-by: Eric Dumazet <edumazet@...gle.com>
Powered by blists - more mailing lists