At 2017-08-10 05:00:19, "Cong Wang" <xiyou.wangc...@gmail.com> wrote: >On Wed, Aug 9, 2017 at 12:17 AM, Gao Feng <gfree.w...@vip.163.com> wrote: >> Hi Cong, >> >> Actually I have one question about the SOCK_RCU_FREE. >> I don't think it could resolve the issue you raised even though it exists >> really. >> >> I checked the SOCK_RCU_FREE, it just defer the __sk_destruct after one rcu >> period. >> But when it performs, someone still could find this sock by callid during >> the del_chan period and it may still deference the sock which may freed soon. >> >> The right flow should be following. >> del_chan() >> wait a rcu period >> sock_put() ------------ It is safe that someone gets the sock because it >> already hold sock refcnt. >> >> When using SOCK_RCU_FREE, its flow would be following. >> wait a rcu period >> del_chan() >> free the sock directly -------- no sock refcnt check again. >> Because the del_chan happens after rcu wait, not before, so it isn't helpful >> with SOCK_RCU_FREE. > > >Yes, good point! With SOCK_RCU_FREE the sock_hold() should >not be needed. For RCU, unpublish should indeed happen before >grace period.
Sorry, I couldn't understand why sock_hold() isn't necessary with SOCK_RCU_FREE. When lookup_chan finds the sock, it would return and reference it later. If no refcnt, how to protect the sock ? Best Regards Feng > > >> >> I don't know if I misunderstand the SOCK_RCU_FREE usage. >> >> But it is a good news that the del_chan is only invoked in pptp_release >> actually and it would wait a rcu period before sock_put. >> > >Looking at the code again, the reader lookup_chan() is actually >invoked in BH context, but neither add_chan() nor del_chan() >actually disables BH...