My overnight testing has not shown any problems with this patch set. JT On Thu, Feb 23, 2017 at 1:29 AM, Jon Maloy <jon.ma...@ericsson.com> wrote:
> 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
