All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Yunsheng Lin <linyunsheng@huawei.com>
Cc: <davem@davemloft.net>, <olteanv@gmail.com>, <ast@kernel.org>,
	<daniel@iogearbox.net>, <andriin@fb.com>, <edumazet@google.com>,
	<weiwan@google.com>, <cong.wang@bytedance.com>,
	<ap420073@gmail.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linuxarm@openeuler.org>,
	<mkl@pengutronix.de>, <linux-can@vger.kernel.org>,
	<jhs@mojatatu.com>, <xiyou.wangcong@gmail.com>,
	<jiri@resnulli.us>, <andrii@kernel.org>, <kafai@fb.com>,
	<songliubraving@fb.com>, <yhs@fb.com>, <john.fastabend@gmail.com>,
	<kpsingh@kernel.org>, <bpf@vger.kernel.org>,
	<jonas.bonn@netrounds.com>, <pabeni@redhat.com>,
	<mzhivich@akamai.com>, <johunt@akamai.com>, <albcamus@gmail.com>,
	<kehuan.feng@gmail.com>, <a.fatoum@pengutronix.de>,
	<atenart@kernel.org>, <alexander.duyck@gmail.com>,
	<hdanton@sina.com>, <jgross@suse.com>, <JKosina@suse.com>,
	<mkubecek@suse.cz>, <bjorn@kernel.org>, <alobakin@pm.me>
Subject: Re: [PATCH net v6 3/3] net: sched: fix tx action reschedule issue with stopped queue
Date: Mon, 10 May 2021 21:22:32 -0700	[thread overview]
Message-ID: <20210510212232.3386c5b4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <1620610956-56306-4-git-send-email-linyunsheng@huawei.com>

On Mon, 10 May 2021 09:42:36 +0800 Yunsheng Lin wrote:
> The netdev qeueue might be stopped when byte queue limit has
> reached or tx hw ring is full, net_tx_action() may still be
> rescheduled endlessly if STATE_MISSED is set, which consumes
> a lot of cpu without dequeuing and transmiting any skb because
> the netdev queue is stopped, see qdisc_run_end().
> 
> This patch fixes it by checking the netdev queue state before
> calling qdisc_run() and clearing STATE_MISSED if netdev queue is
> stopped during qdisc_run(), the net_tx_action() is recheduled
> again when netdev qeueue is restarted, see netif_tx_wake_queue().
> 
> Fixes: 6b3ba9146fe6 ("net: sched: allow qdiscs to handle locking")
> Reported-by: Michal Kubecek <mkubecek@suse.cz>
> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>

Patches 1 and 2 look good to me but this one I'm not 100% sure.

> @@ -251,8 +253,10 @@ static struct sk_buff *dequeue_skb(struct Qdisc *q, bool *validate,
>  	*validate = true;
>  
>  	if ((q->flags & TCQ_F_ONETXQUEUE) &&
> -	    netif_xmit_frozen_or_stopped(txq))
> +	    netif_xmit_frozen_or_stopped(txq)) {
> +		clear_bit(__QDISC_STATE_MISSED, &q->state);
>  		return skb;
> +	}

The queues are woken asynchronously without holding any locks via
netif_tx_wake_queue(). Theoretically we can have a situation where:

CPU 0                            CPU 1   
  .                                .
dequeue_skb()                      .
  netif_xmit_frozen..() # true     .
  .                              [IRQ]
  .                              netif_tx_wake_queue()
  .                              <end of IRQ>
  .                              netif_tx_action()
  .                              set MISSED
  clear MISSED
  return NULL
ret from qdisc_restart()
ret from __qdisc_run()
qdisc_run_end()
-> MISSED not set

  reply	other threads:[~2021-05-11  4:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10  1:42 [PATCH net v6 0/3] fix packet stuck problem for lockless qdisc Yunsheng Lin
2021-05-10  1:42 ` [PATCH net v6 1/3] net: sched: " Yunsheng Lin
2021-05-10  1:42 ` [PATCH net v6 2/3] net: sched: fix endless tx action reschedule during deactivation Yunsheng Lin
2021-05-10  1:42 ` [PATCH net v6 3/3] net: sched: fix tx action reschedule issue with stopped queue Yunsheng Lin
2021-05-11  4:22   ` Jakub Kicinski [this message]
2021-05-11  9:04     ` Yunsheng Lin
2021-05-11 12:13       ` Yunsheng Lin
2021-05-11 23:30         ` Jakub Kicinski
2021-05-12  3:34           ` [Linuxarm] " Yunsheng Lin
2021-05-12 19:47             ` Jakub Kicinski

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=20210510212232.3386c5b4@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=JKosina@suse.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=albcamus@gmail.com \
    --cc=alexander.duyck@gmail.com \
    --cc=alobakin@pm.me \
    --cc=andrii@kernel.org \
    --cc=andriin@fb.com \
    --cc=ap420073@gmail.com \
    --cc=ast@kernel.org \
    --cc=atenart@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=cong.wang@bytedance.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hdanton@sina.com \
    --cc=jgross@suse.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=johunt@akamai.com \
    --cc=jonas.bonn@netrounds.com \
    --cc=kafai@fb.com \
    --cc=kehuan.feng@gmail.com \
    --cc=kpsingh@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@openeuler.org \
    --cc=linyunsheng@huawei.com \
    --cc=mkl@pengutronix.de \
    --cc=mkubecek@suse.cz \
    --cc=mzhivich@akamai.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=songliubraving@fb.com \
    --cc=weiwan@google.com \
    --cc=xiyou.wangcong@gmail.com \
    --cc=yhs@fb.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.