Re: [BUG] act_ife: sleeping functions called in atomic context

[Cong Wang <xiyou.wangcong@gmail.com>]

On Fri, Jun 17, 2016 at 4:07 AM, Jamal Hadi Salim <jhs@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.wangcong@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!