Re: [tipc-discussion] [PATCH v2 net-next 0/6] solve two deadlock issues

[John T. <tho...@gm...>]

Thanks Ying.
I am away for a week and will run some tests next when back.
Regards
John

On 10/03/2017 3:22 AM, "Ying Xue" <yin...@wi...> wrote:

> commit d094c4d5f5 ("tipc: add subscription refcount to avoid invalid
> delete") accidently introduce the following deadlock scenarios:
>
>    CPU1:                             CPU2:
> ----------                     ----------------
> tipc_nametbl_publish
> spin_lock_bh(&tn->nametbl_lock)
> tipc_nametbl_insert_publ
> tipc_nameseq_insert_publ
> tipc_subscrp_report_overlap
> tipc_subscrp_get
> tipc_subscrp_send_event
>                                  tipc_close_conn
>                                  tipc_subscrb_release_cb
>                                  tipc_subscrb_delete
>                                  tipc_subscrp_put
> tipc_subscrp_put
> tipc_subscrp_kref_release
> tipc_nametbl_unsubscribe
> spin_lock_bh(&tn->nametbl_lock)
> <<grab nametbl_lock again>>
>
>    CPU1:                              CPU2:
> ----------                     ----------------
> tipc_nametbl_stop
> spin_lock_bh(&tn->nametbl_lock)
> tipc_purge_publications
> tipc_nameseq_remove_publ
> tipc_subscrp_report_overlap
> tipc_subscrp_get
> tipc_subscrp_send_event
>                                  tipc_close_conn
>                                  tipc_subscrb_release_cb
>                                  tipc_subscrb_delete
>                                  tipc_subscrp_put
> tipc_subscrp_put
> tipc_subscrp_kref_release
> tipc_nametbl_unsubscribe
> spin_lock_bh(&tn->nametbl_lock)
> <<grab nametbl_lock again>>
>
> The root cause of two deadlocks is that we have to hold nametbl lock
> when subscription is freed in tipc_subscrp_kref_release(). In order to
> eliminate the need of taking nametbl lock in
> tipc_subscrp_kref_release(), the functions protected by nametbl lock
> in tipc_subscrp_kref_release() are moved to other places step by step
> in the series.
>
> Change log:
> v2:
>  As Parth's comments, subscription is still present name table after
>  it's expired. To fix it, we introduce a workqueue, and when
>  subscription's timer is expired,  the subscription will be pushed to
>  the workqueue through its work. When the work is scheduled, the
>  subscription be deleted finally.
>
> Ying Xue (6):
>   tipc: advance the time of deleting subscription from
>     subscriber->subscrp_list
>   tipc: adjust the policy of holding subscription kref
>   tipc: adjust policy that sub->timer holds subscription kref
>   tipc: advance the time of calling tipc_nametbl_unsubscribe
>   tipc: remove unnecessary increasement of subscription refcount
>   tipc: delete expired subscription
>
>  net/tipc/name_table.c |  2 ++
>  net/tipc/server.h     |  2 ++
>  net/tipc/subscr.c     | 64 ++++++++++++++++++++++++++++++
> ++++++---------------
>  net/tipc/subscr.h     |  4 ++++
>  4 files changed, 54 insertions(+), 18 deletions(-)
>
> --
> 2.7.4
>
>