From: Cong Wang <xiyou.wangcong@gmail.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: syzbot <syzbot+46f513c3033d592409d2@syzkaller.appspotmail.com>,
David Miller <davem@davemloft.net>,
Jamal Hadi Salim <jhs@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>, Jakub Kicinski <kuba@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
Linux Kernel Network Developers <netdev@vger.kernel.org>,
syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
"Paul E . McKenney" <paulmck@kernel.org>
Subject: Re: WARNING: ODEBUG bug in tcindex_destroy_work (3)
Date: Wed, 25 Mar 2020 11:36:16 -0700 [thread overview]
Message-ID: <CAM_iQpWkNJ+yQ1g+pdkhJBCZ-CJfUGGpvJqOz1JH7LPVtqbRxg@mail.gmail.com> (raw)
In-Reply-To: <875zeuftwm.fsf@nanos.tec.linutronix.de>
On Mon, Mar 23, 2020 at 6:01 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Cong Wang <xiyou.wangcong@gmail.com> writes:
> > On Mon, Mar 23, 2020 at 2:14 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > We use an ordered workqueue for tc filters, so these two
> >> > works are executed in the same order as they are queued.
> >>
> >> The workqueue is ordered, but look how the work is queued on the work
> >> queue:
> >>
> >> tcf_queue_work()
> >> queue_rcu_work()
> >> call_rcu(&rwork->rcu, rcu_work_rcufn);
> >>
> >> So after the grace period elapses rcu_work_rcufn() queues it in the
> >> actual work queue.
> >>
> >> Now tcindex_destroy() is invoked via tcf_proto_destroy() which can be
> >> invoked from preemtible context. Now assume the following:
> >>
> >> CPU0
> >> tcf_queue_work()
> >> tcf_queue_work(&r->rwork, tcindex_destroy_rexts_work);
> >>
> >> -> Migration
> >>
> >> CPU1
> >> tcf_queue_work(&p->rwork, tcindex_destroy_work);
> >>
> >> So your RCU callbacks can be placed on different CPUs which obviously
> >> has no ordering guarantee at all. See also:
> >
> > Good catch!
> >
> > I thought about this when I added this ordered workqueue, but it
> > seems I misinterpret max_active, so despite we have max_active==1,
> > more than 1 work could still be queued on different CPU's here.
>
> The workqueue is not the problem. it works perfectly fine. The way how
> the work gets queued is the issue.
Well, a RCU work is also a work, so the ordered workqueue should
apply to RCU works too, from users' perspective. Users should not
need to learn queue_rcu_work() is actually a call_rcu() which does
not guarantee the ordering for an ordered workqueue.
> > I don't know how to fix this properly, I think essentially RCU work
> > should be guaranteed the same ordering with regular work. But this
> > seems impossible unless RCU offers some API to achieve that.
>
> I don't think that's possible w/o putting constraints on the flexibility
> of RCU (Paul of course might disagree).
>
> I assume that the filters which hang of tcindex_data::perfect and
> tcindex_data:p must be freed before tcindex_data, right?
>
> Refcounting of tcindex_data should do the trick. I.e. any element which
> you add to a tcindex_data instance takes a refcount and when that is
> destroyed then the rcu/work callback drops a reference which once it
> reaches 0 triggers tcindex_data to be freed.
Yeah, but the problem is more than just tcindex filter, we have many
places make the same assumption of ordering.
Thanks!
next prev parent reply other threads:[~2020-03-25 18:36 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-25 5:43 WARNING: ODEBUG bug in tcindex_destroy_work (3) syzbot
2020-03-16 23:47 ` syzbot
2020-03-21 10:19 ` Thomas Gleixner
2020-03-23 17:48 ` Cong Wang
2020-03-23 21:14 ` Thomas Gleixner
2020-03-23 23:14 ` Cong Wang
2020-03-24 1:01 ` Thomas Gleixner
2020-03-24 2:05 ` Paul E. McKenney
2020-03-25 18:53 ` Cong Wang
2020-03-25 19:07 ` Paul E. McKenney
2020-03-25 18:36 ` Cong Wang [this message]
2020-03-25 18:58 ` Paul E. McKenney
2020-03-28 19:53 ` Cong Wang
2020-03-30 13:37 ` Paul E. McKenney
2020-03-21 4:49 ` syzbot
2020-03-21 5:42 ` Dominik Brodowski
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=CAM_iQpWkNJ+yQ1g+pdkhJBCZ-CJfUGGpvJqOz1JH7LPVtqbRxg@mail.gmail.com \
--to=xiyou.wangcong@gmail.com \
--cc=davem@davemloft.net \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=syzbot+46f513c3033d592409d2@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tglx@linutronix.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).