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

On Fri, Jun 17, 2016 at 4:07 AM, Jamal Hadi Salim [off-list ref] wrote:

On 16-06-17 01:38 AM, Cong Wang wrote:

quoted

On Thu, Jun 16, 2016 at 7:14 PM, Cong Wang [off-list ref]
wrote:

quoted

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!