All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joe Damato <jdamato@fastly.com>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: netdev@vger.kernel.org, kuba@kernel.org, davem@davemloft.net,
	hawk@kernel.org
Subject: Re: [PATCH net-next 0/6] net: page_pool: Add page_pool stat counters
Date: Sat, 29 Jan 2022 10:07:41 -0800	[thread overview]
Message-ID: <CALALjgzX+=rRTokT_k8grt38fJXUa+=Ed+KCkHz=BhkEquDF0w@mail.gmail.com> (raw)
In-Reply-To: <YfVKDxenS5IWxCLX@hades>

On Sat, Jan 29, 2022 at 6:07 AM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Joe!
>
> On Thu, Jan 27, 2022 at 03:55:03PM -0800, Joe Damato wrote:
> > On Thu, Jan 27, 2022 at 1:08 AM Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Joe,
> > >
> > > On Wed, Jan 26, 2022 at 02:48:14PM -0800, Joe Damato wrote:
> > > > Greetings:
> > > >
> > > > This series adds some stat counters for the page_pool allocation path which
> > > > help to track:
> > > >
> > > >       - fast path allocations
> > > >       - slow path order-0 allocations
> > > >       - slow path high order allocations
> > > >       - refills which failed due to an empty ptr ring, forcing a slow
> > > >         path allocation
> > > >       - allocations fulfilled via successful refill
> > > >       - pages which cannot be added to the cache because of numa mismatch
> > > >         (i.e. waived)
> > > >
> > >
> > > Thanks for the patch.  Stats are something that's indeed missing from the
> > > API.  The patch  should work for Rx based allocations (which is what you
> > > currently cover),  since the RX side is usually protected by NAPI.  However
> > > we've added a few features recently,  which we would like to have stats on.
> >
> > Thanks for taking a look at the patch.
> >
>
> yw
>
> > > commit 6a5bcd84e886("page_pool: Allow drivers to hint on SKB recycling"),
> > > introduces recycling capabilities on the API.  I think it would be far more
> > > interesting to be able to extend the statistics to recycled/non-recycled
> > > packets as well in the future.
> >
> > I agree. Tracking recycling events would be both helpful and
> > interesting, indeed.
> >
> > > But the recycling is asynchronous and we
> > > can't add locks just for the sake of accurate statistics.
> >
> > Agreed.
> >
> > > Can we instead
> > > convert that to a per-cpu structure for producers?
> >
> > If my understanding of your proposal is accurate, moving the stats
> > structure to a per-cpu structure (instead of per-pool) would add
> > ambiguity as to the performance of a specific driver's page pool. In
> > exchange for the ambiguity, though, we'd get stats for additional
> > events, which could be interesting.
>
> I was mostly thinking per pool using with 'struct percpu_counter' or
> allocate __percpu variables,  but I haven't really checked if that's doable or
> which of those is better suited for our case.

I wrote up a v2 last night that allocates and exports a
page_pool_stats structure per cpu (but not per pool). The data can be
accessed by users in the file /proc/net/page_pool_stats. The approach
is similar to the way softnet_stat is implemented.

The main advantage with this approach is that no driver modifications
are needed and no additional APIs are exposed that will need to be
maintained. Adding new stats in the future would be much simpler with
this approach. I've also moved all the code behind a kernel config
flag so users can opt-in to get these stats.

I'll send the v2 shortly.

Thanks,
Joe

      reply	other threads:[~2022-01-29 18:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26 22:48 [PATCH net-next 0/6] net: page_pool: Add page_pool stat counters Joe Damato
2022-01-26 22:48 ` [PATCH 1/6] net: page_pool: Add alloc stats and fast path stat Joe Damato
2022-01-27 16:32   ` Jakub Kicinski
2022-01-27 21:11     ` Joe Damato
2022-02-02 14:14   ` Jesper Dangaard Brouer
2022-01-26 22:48 ` [PATCH net-next 2/6] net: page_pool: Add a stat for the slow alloc path Joe Damato
2022-01-26 22:48 ` [PATCH net-next 3/6] net: page_pool: Add a high order alloc stat Joe Damato
2022-01-26 22:48 ` [PATCH net-next 4/6] net: page_pool: Add stat tracking empty ring Joe Damato
2022-01-26 22:48 ` [PATCH net-next 5/6] net: page_pool: Add stat tracking cache refills Joe Damato
2022-01-26 22:48 ` [PATCH 6/6] net: page_pool: Add a stat tracking waived pages Joe Damato
2022-01-27  8:51 ` [PATCH net-next 0/6] net: page_pool: Add page_pool stat counters Jesper Dangaard Brouer
2022-01-27 21:08   ` Joe Damato
2022-02-02 14:04     ` Jesper Dangaard Brouer
2022-01-27  9:08 ` Ilias Apalodimas
2022-01-27 23:55   ` Joe Damato
2022-01-29 14:07     ` Ilias Apalodimas
2022-01-29 18:07       ` Joe Damato [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALALjgzX+=rRTokT_k8grt38fJXUa+=Ed+KCkHz=BhkEquDF0w@mail.gmail.com' \
    --to=jdamato@fastly.com \
    --cc=davem@davemloft.net \
    --cc=hawk@kernel.org \
    --cc=ilias.apalodimas@linaro.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.