All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Xin Zhao <jackzxcui1989@163.com>,
	 willemdebruijn.kernel@gmail.com,  edumazet@google.com,
	 ferenc@fejes.dev
Cc: davem@davemloft.net,  kuba@kernel.org,  pabeni@redhat.com,
	 horms@kernel.org,  netdev@vger.kernel.org,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v9] net: af_packet: Use hrtimer to do the retire operation
Date: Fri, 29 Aug 2025 14:01:14 -0400	[thread overview]
Message-ID: <willemdebruijn.kernel.3427046386018@gmail.com> (raw)
In-Reply-To: <20250829140539.3840337-1-jackzxcui1989@163.com>

Xin Zhao wrote:
> On Fri, 2025-08-29 at 7:16 +0800, Willem wrote:
> 
> > Overall I'm on-board with this version.
> > 
> > 
> > One remaining question is the intended behavior of kactive_blk_num (K)
> > and last_kactive_blk_num (L).
> > 
> > K is incremented on block close. L is set to match K on block open and
> > each timer. So the only time that they differ is if a block is closed
> > in tpacket_rcv and no new block could be opened.
> > 
> > The only use of L is that the core of the timer callback is skipped if
> > L != K. Based on the above that can only happen if a block was closed
> > in tpacket_rcv and no new block could be opened (because the ring is
> > full), so the ring is frozen. So it only skips the frozen part of the
> > timer callback. Until the next timeout. But why? If the queue is
> > frozen and the next block is no longer in use, just call
> > prb_open_block right away?
> > 
> > Unless I'm missing something, I think we can make that simplification.
> > Then we won't have to worry that the behavior changes after this
> > patch. It should be a separate precursor patch though.
> 
> 
> I followed what you said, I think we can directly remove the
> last_kactive_blk_num variable and all related logic. I have run the
> optimized code in our project and haven't encountered any issues.
> The only impact I can think of is that in tpacket_rcv, when discovering
> that the next block is still in use and cannot be opened, we originally
> needed to wait for interval time in the timer callback before checking
> again. Now, we will make that check earlier,

At the next timer callback, which might run immediately.

But importantly averaged it is at half the timer latency, which
is in the same ballpark.

> which may allow us to
> discover that the user has finished using the block sooner and open the
> block earlier. So it seems that the optimization does not cause any issues.
> 
> 
> 
> So what should I do next?
> 
> I submit a patch that removes last_kactive_blk_num, then wait the patch
> merged, and after that rebase to submit the current hrtimer patch?
> 
> or
> 
> I wait for you to submit the patch that removes last_kactive_blk_num, and
> then rebase to submit the new hrtimer patch?
> 
> or
> 
> I directly include this modification in the hrtimer patch in version 10?

My idea was 4: you send a patch series with both patches.

But either 1 or 2 are fine too.

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

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-29 14:05 [PATCH net-next v9] net: af_packet: Use hrtimer to do the retire operation Xin Zhao
2025-08-29 18:01 ` Willem de Bruijn [this message]
  -- strict thread matches above, loose matches on Subject: below --
2025-08-28 15:51 Xin Zhao
2025-08-28 23:16 ` Willem de Bruijn

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=willemdebruijn.kernel.3427046386018@gmail.com \
    --to=willemdebruijn.kernel@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ferenc@fejes.dev \
    --cc=horms@kernel.org \
    --cc=jackzxcui1989@163.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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.