On Sun, Feb 14, 2021 at 2:56 AM Taehee Yoo <ap420...@gmail.com> wrote: > > > > On 21. 2. 14. 오전 4:07, Cong Wang wrote: > > On Sat, Feb 13, 2021 at 9:51 AM Taehee Yoo <ap420...@gmail.com> wrote: > >> -static void mld_dad_start_timer(struct inet6_dev *idev, unsigned > long delay) > >> +static void mld_dad_start_work(struct inet6_dev *idev, unsigned > long delay) > >> { > >> unsigned long tv = prandom_u32() % delay; > >> > >> - if (!mod_timer(&idev->mc_dad_timer, jiffies+tv+2)) > >> + if (!mod_delayed_work(mld_wq, &idev->mc_dad_work, > msecs_to_jiffies(tv + 2))) > > > > IIUC, before this patch 'delay' is in jiffies, after this patch it is > in msecs? > > > > Ah, I understand, It's my mistake. > I didn't change the behavior of 'delay' in this patchset. > So, 'delay' is still in jiffies, not msecs. > Therefore, msecs_to_jiffies() should not be used in this patchset. > I will send a v3 patch, which doesn't use msecs_to_jiffies(). > Thanks! > > By the way, I think the 'delay' is from the > unsolicited_report_interval() and it just return value of > idev->cnf.mldv{1 | 2}_unsolicited_report_interval. > I think this value is msecs, not jiffies. > So, It should be converted to use msecs_to_jiffies(), I think. > How do you think about it?
Hmm? I think it is in jiffies: .mldv1_unsolicited_report_interval = 10 * HZ, .mldv2_unsolicited_report_interval = HZ, > > > [...] > > > >> -static void mld_dad_timer_expire(struct timer_list *t) > >> +static void mld_dad_work(struct work_struct *work) > >> { > >> - struct inet6_dev *idev = from_timer(idev, t, mc_dad_timer); > >> + struct inet6_dev *idev = container_of(to_delayed_work(work), > >> + struct inet6_dev, > >> + mc_dad_work); > >> > >> + rtnl_lock(); > > > > Any reason why we need RTNL after converting the timer to > > delayed work? > > > > For the moment, RTNL is not needed. > But the Resources, which are used by delayed_work will be protected by > RTNL instead of other locks. > So, It just pre-adds RTNL and the following patches will delete other locks. Sounds like this change does not belong to this patch. ;) If so, please move it to where ever more appropriate. Thanks.