Re: [PATCH net] page_pool: store detach_time as ktime_t to avoid false-negatives

[Jesper Dangaard Brouer <hawk@kernel.org>]

On 07/03/2026 21.40, Jakub Kicinski wrote:
> While testing other changes in vng I noticed that
> nl_netdev.page_pool_check flakes. This never happens in real CI.
> 
> Turns out vng may boot and get to that test in less than a second.
> page_pool_detached() records the detach time in seconds, so if
> vng is fast enough detach time is set to 0. Other code treats
> 0 as "not detached". detach_time is only used to report the state
> to the user, so it's not a huge deal in practice but let's fix it.
> Store the raw ktime_t (nanoseconds) instead. A nanosecond value
> of 0 is practically impossible.
> 
> Fixes: 69cb4952b6f6 ("net: page_pool: report when page pool was destroyed")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: hawk@kernel.org

LGTM -- after addressing in build errors... likely a missing include?

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>


> CC: ilias.apalodimas@linaro.org
> ---
>   include/net/page_pool/types.h | 2 +-
>   net/core/page_pool_user.c     | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index 0d453484a585..cdd95477af7a 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -247,7 +247,7 @@ struct page_pool {
>   	/* User-facing fields, protected by page_pools_lock */
>   	struct {
>   		struct hlist_node list;
> -		u64 detach_time;
> +		ktime_t detach_time;
>   		u32 id;
>   	} user;
>   };
> diff --git a/net/core/page_pool_user.c b/net/core/page_pool_user.c
> index c82a95beceff..fb4ca27358cd 100644
> --- a/net/core/page_pool_user.c
> +++ b/net/core/page_pool_user.c
> @@ -245,7 +245,7 @@ page_pool_nl_fill(struct sk_buff *rsp, const struct page_pool *pool,
>   		goto err_cancel;
>   	if (pool->user.detach_time &&
>   	    nla_put_uint(rsp, NETDEV_A_PAGE_POOL_DETACH_TIME,
> -			 pool->user.detach_time))
> +			 ktime_to_seconds(pool->user.detach_time)))

Hmm.. is this ktime_to_seconds even defined?

A quick look tells me that we could use:

  ktime_divns(pool->user.detach_time, NSEC_PER_SEC)


>   		goto err_cancel;
>   
>   	if (pool->mp_ops && pool->mp_ops->nl_fill(pool->mp_priv, rsp, NULL))
> @@ -337,7 +337,7 @@ int page_pool_list(struct page_pool *pool)
>   void page_pool_detached(struct page_pool *pool)
>   {
>   	mutex_lock(&page_pools_lock);
> -	pool->user.detach_time = ktime_get_boottime_seconds();
> +	pool->user.detach_time = ktime_get_boottime();
>   	netdev_nl_page_pool_event(pool, NETDEV_CMD_PAGE_POOL_CHANGE_NTF);
>   	mutex_unlock(&page_pools_lock);
>   }