On Fri, Jun 17, 2016 at 4:07 AM, Jamal Hadi Salim <j...@mojatatu.com> wrote: > On 16-06-17 01:38 AM, Cong Wang wrote: >> >> On Thu, Jun 16, 2016 at 7:14 PM, Cong Wang <xiyou.wangc...@gmail.com> >> wrote: >>> >>> >>> I think we can just remove that tcf_lock, I am testing a patch now. >> >> >> Please try the attached patch, I will do more tests tomorrow. >> >> Thanks! >> > > Cong, What tree are you using? I dont see the time aggregation patches > that I sent (and Dave took in) in your changes.
My patch is against -net. (I see you already figured out your patch is missing in -net-next.) Or are you suggesting to rebase it for -net-next? I think it fixes some real bug so -net is better, although it is slightly large as a bug fix. > > Comments: > Is GFP_ATOMIC really necessary? Thats user->kernel interface. GFP_KERNEL > should be sufficient. I added a read_lock(ife_mod_lock), this is why we need GFP_ATOMIC. Again, don't worry, this change should be in a separated patch, you will not miss it again when I send them formally. ;) > Also, it would be nice to kill the lock - but this feels like two > patches in one. 1) to fix the alloc not to be under the lock 2) to > kill said lock. Maybe split it as such for easier review. > I am using this action extensively so will be happy to test. > I think my patch is a good beginning to #1 - if you fix the forgotten > unlock and ensure we lock around updating ife fields when it exists > already (you said it in your earlier email and I thought about > that afterwards). Yes, it makes sense too. Your patch is smaller, if you plan to backport it to stable, we can use your patch for -net and -stable and I am happy to rebase mine for -net-next. I am fine with either way. Thanks!