Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: WARNING: ODEBUG bug in tcindex_destroy_work (3) | Date | Tue, 24 Mar 2020 02:01:13 +0100 |
| |
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.
> 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.
Thanks,
tglx
| |