On Wed, Sep 12, 2018 at 1:51 AM Vlad Buslov <vla...@mellanox.com> wrote: > > > On Fri 07 Sep 2018 at 19:12, Cong Wang <xiyou.wangc...@gmail.com> wrote: > > On Fri, Sep 7, 2018 at 6:52 AM Vlad Buslov <vla...@mellanox.com> wrote: > >> > >> Action API was changed to work with actions and action_idr in concurrency > >> safe manner, however tcf_del_walker() still uses actions without taking a > >> reference or idrinfo->lock first, and deletes them directly, disregarding > >> possible concurrent delete. > >> > >> Add tc_action_wq workqueue to action API. Implement > >> tcf_idr_release_unsafe() that assumes external synchronization by caller > >> and delays blocking action cleanup part to tc_action_wq workqueue. Extend > >> tcf_action_cleanup() with 'async' argument to indicate that function should > >> free action asynchronously. > > > > Where exactly is blocking in tcf_action_cleanup()? > > > > From your code, it looks like free_tcf(), but from my observation, > > the only blocking function inside is tcf_action_goto_chain_fini() > > which calls __tcf_chain_put(). But, __tcf_chain_put() is blocking > > _ONLY_ when tc_chain_notify() is called, for tc action it is never > > called. > > > > So, what else is blocking? > > __tcf_chain_put() calls tc_chain_tmplt_del(), which calls > ops->tmplt_destroy(). This last function uses hw offload API, which is > blocking.
Good to know. Can we just make ops->tmplt_destroy() to use workqueue? Making tc action to workqueue seems overkill, for me.
