Ok. So go for it to net-next. Reviewed-by: Jon ///jon > -----Original Message----- > From: Ying Xue [mailto:ying....@windriver.com] > Sent: Wednesday, February 22, 2017 06:47 AM > To: Jon Maloy <jon.ma...@ericsson.com>; Parthasarathy Bhuvaragan > <parthasarathy.bhuvara...@ericsson.com>; thompa....@gmail.com > Cc: tipc-discussion@lists.sourceforge.net > Subject: Re: [tipc-discussion] [net 0/5] solve two deadlock issues > > At least, net-next tree is still open as David is reviewing patches submitted > to > net-next. > > Hope we have a window to submit the series to net-next. > > Thanks, > Ying > > On 02/22/2017 07:42 PM, Xue, Ying wrote: > > Hi Jon, > > > > I understood your concern. > > > > I have checked the possibility of merging patch #1, #4 and #5 as one. > However, just merging the three patch is insufficient, and at least #2 seems > necessary too, otherwise, another deadlock still exists due to two premature > 'return's in subcsrb_report_overlap(). Even if we merged them as one, it will > lose my initial purpose of dividing the series as so small patches. Although > each patch is made a small change, it's often related to a policy adjustment > of > locking or holding refcount. Moreover, as our locking policy associated with > topserver becomes complex, I want to use the comments in each patch > header to record what policy has been adjusted. In the future, the > information can guide whether our changes comply with the adjusted policy > or not. > > > > In fact, all changes contained in the series is not big. But if we merged > > them > as one, all useful messages will be lost forever. > > > > Additionally, "net-next" tree reaches 4.10-rc8, and "net" tree is 4.10-rc7 > now. I saw today there was one developer who submitted a patch to net- > next and David also accepted it. However, if John's testing proved the series > is okay tomorrow, probably I can send the series to net-next tomorrow. Even > for the worst case, we cannot submit the series until net-next is open again. > But I have checked nobody would maintain 4.10 as a stable version. So even > if there is a big long time gap, it seems not to cause a series issue. > > > > Regards, > > Ying > > > > -----Original Message----- > > From: Jon Maloy [mailto:jon.ma...@ericsson.com] > > Sent: Tuesday, February 21, 2017 7:12 PM > > To: Xue, Ying; Parthasarathy Bhuvaragan; thompa....@gmail.com > > Cc: tipc-discussion@lists.sourceforge.net > > Subject: RE: [net 0/5] solve two deadlock issues > > > > Hi Ying, > > These are good design changes, that definitely should go in asap. However, > I feel deeply uncomfortable with such a big change going into 'net', > especially > since our previous, exceptionally large, contribution now has turned out to > have problems. I wonder if we could not get away with something simpler > for 'net'. > > > > Looking closer at your series, it seems to me that only patches ## 1, 4, and > the lock removal part of #5 are really needed to solve the problem we have > at hand now. Why not merge those into one patch and deliver this to 'net', > while reference count redesign parts can go into net-next ? > > > > Regards > > ///jon > > > > > >> -----Original Message----- > >> From: Ying Xue [mailto:ying....@windriver.com] > >> Sent: Monday, February 20, 2017 06:39 AM > >> To: Jon Maloy <jon.ma...@ericsson.com>; Parthasarathy Bhuvaragan > >> <parthasarathy.bhuvara...@ericsson.com>; thompa....@gmail.com > >> Cc: tipc-discussion@lists.sourceforge.net > >> Subject: [net 0/5] solve two deadlock issues > >> > >> 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. > >> > >> Ying Xue (5): > >> 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 > >> > >> net/tipc/name_table.c | 2 ++ > >> net/tipc/subscr.c | 32 ++++++++++++++------------------ > >> net/tipc/subscr.h | 3 +++ > >> 3 files changed, 19 insertions(+), 18 deletions(-) > >> > >> -- > >> 2.7.4 > > > > > > ---------------------------------------------------------------------- > > -------- Check out the vibrant tech community on one of the world's > > most engaging tech sites, SlashDot.org! http://sdm.link/slashdot > > _______________________________________________ > > tipc-discussion mailing list > > tipc-discussion@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/tipc-discussion > >
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ tipc-discussion mailing list tipc-discussion@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tipc-discussion
