Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thu, Feb 8, 2018 at 2:41 PM, Jia-Ju Bai  wrote:
>
>
> On 2018/2/8 18:20, Rafael J. Wysocki wrote:
>>
>> On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:
>>>
>>> Quoting Rafael J. Wysocki (2018-02-08 09:51:41)

 On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
>
> After checking all possible call chains to acpi_os_execute here,
> my tool finds that acpi_os_execute is never called in atomic context.
> And acpi_os_execute calls acpi_debugger_create_thread
> which calls mutex_lock,
> thus it proves again that acpi_os_execute can
> call functions which may sleep.
> Thus GFP_ATOMIC is not necessary, and it can be replaced with
> GFP_KERNEL.
>
> This is found by a static analysis tool named DCNS written by myself.
>
> Signed-off-by: Jia-Ju Bai 
> ---
>   drivers/acpi/osl.c |2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb..8ee605e 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type
> type,
> * having a static work_struct.
> */
>   - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
>if (!dpc)
>return AE_NO_MEMORY;
>

 Applied, thanks!
>>>
>>> Hmm, not this patch per se, but
>>>
>>> https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
>>> [  111.378236] BUG: sleeping function called from invalid context at
>>> mm/slab.h:420
>>> [  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name:
>>> gem_exec_flush
>>> [  111.378275] INFO: lockdep is turned off.
>>> [  111.378277] irq event stamp: 0
>>> [  111.378280] hardirqs last  enabled at (0): [<  (null)>]
>>> (null)
>>> [  111.378286] hardirqs last disabled at (0): []
>>> copy_process.part.7+0x2f1/0x1db0
>>> [  111.378290] softirqs last  enabled at (0): []
>>> copy_process.part.7+0x2f1/0x1db0
>>> [  111.378292] softirqs last disabled at (0): [<  (null)>]
>>> (null)
>>> [  111.378293] Preemption disabled at:
>>> [  111.378298] [] __mutex_lock+0x56/0x9b0
>>> [  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
>>> 4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
>>> [  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A,
>>> BIOS APLKRVPA.X64.0150.B11.1608081044 08/08/2016
>>> [  111.378314] Call Trace:
>>> [  111.378318]  
>>> [  111.378323]  dump_stack+0x5f/0x86
>>> [  111.378328]  ___might_sleep+0x1d9/0x240
>>> [  111.378334]  ? acpi_os_execute+0x2d/0x130
>>> [  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
>>> [  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
>>> [  111.378347]  acpi_os_execute+0x2d/0x130
>>> [  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
>>> [  111.378355]  acpi_ev_gpe_detect+0x156/0x195
>>> [  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
>>> [  111.378365]  acpi_irq+0xd/0x30
>>> [  111.378369]  __handle_irq_event_percpu+0x3c/0x340
>>> [  111.378374]  handle_irq_event_percpu+0x1b/0x50
>>> [  111.378378]  handle_irq_event+0x2f/0x50
>>> [  111.378381]  handle_fasteoi_irq+0x93/0x150
>>> [  111.378386]  handle_irq+0x11/0x20
>>> [  111.378390]  do_IRQ+0x5e/0x120
>>> [  111.378395]  common_interrupt+0xbb/0xbb
>>> [  111.378397]  
>>>
>>> does tell us that acpi_os_execute() is called in irq context.
>>
>> Well, right, thanks!
>>
>> I overlooked this instance, so dropping the patch.
>
>
> Sorry for my false positive.

No worries.

> My tool missed that acpi_ev_sci_xrupt_handler() is an interrupt handler, 
> sorry.

That just means that the tool is not perfect, which is nothing unusual. :-)

I only would suggest double checking its findings before sending out
patches next time.

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thu, Feb 8, 2018 at 2:41 PM, Jia-Ju Bai  wrote:
>
>
> On 2018/2/8 18:20, Rafael J. Wysocki wrote:
>>
>> On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:
>>>
>>> Quoting Rafael J. Wysocki (2018-02-08 09:51:41)

 On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
>
> After checking all possible call chains to acpi_os_execute here,
> my tool finds that acpi_os_execute is never called in atomic context.
> And acpi_os_execute calls acpi_debugger_create_thread
> which calls mutex_lock,
> thus it proves again that acpi_os_execute can
> call functions which may sleep.
> Thus GFP_ATOMIC is not necessary, and it can be replaced with
> GFP_KERNEL.
>
> This is found by a static analysis tool named DCNS written by myself.
>
> Signed-off-by: Jia-Ju Bai 
> ---
>   drivers/acpi/osl.c |2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb..8ee605e 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type
> type,
> * having a static work_struct.
> */
>   - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
>if (!dpc)
>return AE_NO_MEMORY;
>

 Applied, thanks!
>>>
>>> Hmm, not this patch per se, but
>>>
>>> https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
>>> [  111.378236] BUG: sleeping function called from invalid context at
>>> mm/slab.h:420
>>> [  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name:
>>> gem_exec_flush
>>> [  111.378275] INFO: lockdep is turned off.
>>> [  111.378277] irq event stamp: 0
>>> [  111.378280] hardirqs last  enabled at (0): [<  (null)>]
>>> (null)
>>> [  111.378286] hardirqs last disabled at (0): []
>>> copy_process.part.7+0x2f1/0x1db0
>>> [  111.378290] softirqs last  enabled at (0): []
>>> copy_process.part.7+0x2f1/0x1db0
>>> [  111.378292] softirqs last disabled at (0): [<  (null)>]
>>> (null)
>>> [  111.378293] Preemption disabled at:
>>> [  111.378298] [] __mutex_lock+0x56/0x9b0
>>> [  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
>>> 4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
>>> [  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A,
>>> BIOS APLKRVPA.X64.0150.B11.1608081044 08/08/2016
>>> [  111.378314] Call Trace:
>>> [  111.378318]  
>>> [  111.378323]  dump_stack+0x5f/0x86
>>> [  111.378328]  ___might_sleep+0x1d9/0x240
>>> [  111.378334]  ? acpi_os_execute+0x2d/0x130
>>> [  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
>>> [  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
>>> [  111.378347]  acpi_os_execute+0x2d/0x130
>>> [  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
>>> [  111.378355]  acpi_ev_gpe_detect+0x156/0x195
>>> [  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
>>> [  111.378365]  acpi_irq+0xd/0x30
>>> [  111.378369]  __handle_irq_event_percpu+0x3c/0x340
>>> [  111.378374]  handle_irq_event_percpu+0x1b/0x50
>>> [  111.378378]  handle_irq_event+0x2f/0x50
>>> [  111.378381]  handle_fasteoi_irq+0x93/0x150
>>> [  111.378386]  handle_irq+0x11/0x20
>>> [  111.378390]  do_IRQ+0x5e/0x120
>>> [  111.378395]  common_interrupt+0xbb/0xbb
>>> [  111.378397]  
>>>
>>> does tell us that acpi_os_execute() is called in irq context.
>>
>> Well, right, thanks!
>>
>> I overlooked this instance, so dropping the patch.
>
>
> Sorry for my false positive.

No worries.

> My tool missed that acpi_ev_sci_xrupt_handler() is an interrupt handler, 
> sorry.

That just means that the tool is not perfect, which is nothing unusual. :-)

I only would suggest double checking its findings before sending out
patches next time.

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Jia-Ju Bai 



On 2018/2/8 18:20, Rafael J. Wysocki wrote:

On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:

Quoting Rafael J. Wysocki (2018-02-08 09:51:41)

On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:

After checking all possible call chains to acpi_os_execute here,
my tool finds that acpi_os_execute is never called in atomic context.
And acpi_os_execute calls acpi_debugger_create_thread
which calls mutex_lock,
thus it proves again that acpi_os_execute can
call functions which may sleep.
Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai 
---
  drivers/acpi/osl.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb..8ee605e 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
* having a static work_struct.
*/
  
- dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);

+ dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
   if (!dpc)
   return AE_NO_MEMORY;
  


Applied, thanks!

Hmm, not this patch per se, but

https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
[  111.378236] BUG: sleeping function called from invalid context at 
mm/slab.h:420
[  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
gem_exec_flush
[  111.378275] INFO: lockdep is turned off.
[  111.378277] irq event stamp: 0
[  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
(null)
[  111.378286] hardirqs last disabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378290] softirqs last  enabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378292] softirqs last disabled at (0): [<  (null)>]   
(null)
[  111.378293] Preemption disabled at:
[  111.378298] [] __mutex_lock+0x56/0x9b0
[  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
[  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
APLKRVPA.X64.0150.B11.1608081044 08/08/2016
[  111.378314] Call Trace:
[  111.378318]  
[  111.378323]  dump_stack+0x5f/0x86
[  111.378328]  ___might_sleep+0x1d9/0x240
[  111.378334]  ? acpi_os_execute+0x2d/0x130
[  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
[  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
[  111.378347]  acpi_os_execute+0x2d/0x130
[  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
[  111.378355]  acpi_ev_gpe_detect+0x156/0x195
[  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
[  111.378365]  acpi_irq+0xd/0x30
[  111.378369]  __handle_irq_event_percpu+0x3c/0x340
[  111.378374]  handle_irq_event_percpu+0x1b/0x50
[  111.378378]  handle_irq_event+0x2f/0x50
[  111.378381]  handle_fasteoi_irq+0x93/0x150
[  111.378386]  handle_irq+0x11/0x20
[  111.378390]  do_IRQ+0x5e/0x120
[  111.378395]  common_interrupt+0xbb/0xbb
[  111.378397]  

does tell us that acpi_os_execute() is called in irq context.

Well, right, thanks!

I overlooked this instance, so dropping the patch.


Sorry for my false positive.
My tool missed that acpi_ev_sci_xrupt_handler() is an interrupt handler, 
sorry.

Thank Chris for testing my patch.


Best wishes,
Jia-Ju Bai

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Jia-Ju Bai 



On 2018/2/8 18:20, Rafael J. Wysocki wrote:

On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:

Quoting Rafael J. Wysocki (2018-02-08 09:51:41)

On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:

After checking all possible call chains to acpi_os_execute here,
my tool finds that acpi_os_execute is never called in atomic context.
And acpi_os_execute calls acpi_debugger_create_thread
which calls mutex_lock,
thus it proves again that acpi_os_execute can
call functions which may sleep.
Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai 
---
  drivers/acpi/osl.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 3bb46cb..8ee605e 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
* having a static work_struct.
*/
  
- dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);

+ dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
   if (!dpc)
   return AE_NO_MEMORY;
  


Applied, thanks!

Hmm, not this patch per se, but

https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
[  111.378236] BUG: sleeping function called from invalid context at 
mm/slab.h:420
[  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
gem_exec_flush
[  111.378275] INFO: lockdep is turned off.
[  111.378277] irq event stamp: 0
[  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
(null)
[  111.378286] hardirqs last disabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378290] softirqs last  enabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378292] softirqs last disabled at (0): [<  (null)>]   
(null)
[  111.378293] Preemption disabled at:
[  111.378298] [] __mutex_lock+0x56/0x9b0
[  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
[  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
APLKRVPA.X64.0150.B11.1608081044 08/08/2016
[  111.378314] Call Trace:
[  111.378318]  
[  111.378323]  dump_stack+0x5f/0x86
[  111.378328]  ___might_sleep+0x1d9/0x240
[  111.378334]  ? acpi_os_execute+0x2d/0x130
[  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
[  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
[  111.378347]  acpi_os_execute+0x2d/0x130
[  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
[  111.378355]  acpi_ev_gpe_detect+0x156/0x195
[  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
[  111.378365]  acpi_irq+0xd/0x30
[  111.378369]  __handle_irq_event_percpu+0x3c/0x340
[  111.378374]  handle_irq_event_percpu+0x1b/0x50
[  111.378378]  handle_irq_event+0x2f/0x50
[  111.378381]  handle_fasteoi_irq+0x93/0x150
[  111.378386]  handle_irq+0x11/0x20
[  111.378390]  do_IRQ+0x5e/0x120
[  111.378395]  common_interrupt+0xbb/0xbb
[  111.378397]  

does tell us that acpi_os_execute() is called in irq context.

Well, right, thanks!

I overlooked this instance, so dropping the patch.


Sorry for my false positive.
My tool missed that acpi_ev_sci_xrupt_handler() is an interrupt handler, 
sorry.

Thank Chris for testing my patch.


Best wishes,
Jia-Ju Bai

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:
> Quoting Rafael J. Wysocki (2018-02-08 09:51:41)
> > On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> > > After checking all possible call chains to acpi_os_execute here,
> > > my tool finds that acpi_os_execute is never called in atomic context.
> > > And acpi_os_execute calls acpi_debugger_create_thread 
> > > which calls mutex_lock,
> > > thus it proves again that acpi_os_execute can 
> > > call functions which may sleep.
> > > Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> > > 
> > > This is found by a static analysis tool named DCNS written by myself.
> > > 
> > > Signed-off-by: Jia-Ju Bai 
> > > ---
> > >  drivers/acpi/osl.c |2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > > index 3bb46cb..8ee605e 100644
> > > --- a/drivers/acpi/osl.c
> > > +++ b/drivers/acpi/osl.c
> > > @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
> > >* having a static work_struct.
> > >*/
> > >  
> > > - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> > > + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
> > >   if (!dpc)
> > >   return AE_NO_MEMORY;
> > >  
> > > 
> > 
> > Applied, thanks!
> 
> Hmm, not this patch per se, but 
> 
> https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
> [  111.378236] BUG: sleeping function called from invalid context at 
> mm/slab.h:420
> [  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
> gem_exec_flush
> [  111.378275] INFO: lockdep is turned off.
> [  111.378277] irq event stamp: 0
> [  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
> (null)
> [  111.378286] hardirqs last disabled at (0): [] 
> copy_process.part.7+0x2f1/0x1db0
> [  111.378290] softirqs last  enabled at (0): [] 
> copy_process.part.7+0x2f1/0x1db0
> [  111.378292] softirqs last disabled at (0): [<  (null)>]   
> (null)
> [  111.378293] Preemption disabled at:
> [  111.378298] [] __mutex_lock+0x56/0x9b0
> [  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W  
>   4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
> [  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
> APLKRVPA.X64.0150.B11.1608081044 08/08/2016
> [  111.378314] Call Trace:
> [  111.378318]  
> [  111.378323]  dump_stack+0x5f/0x86
> [  111.378328]  ___might_sleep+0x1d9/0x240
> [  111.378334]  ? acpi_os_execute+0x2d/0x130
> [  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
> [  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
> [  111.378347]  acpi_os_execute+0x2d/0x130
> [  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
> [  111.378355]  acpi_ev_gpe_detect+0x156/0x195
> [  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
> [  111.378365]  acpi_irq+0xd/0x30
> [  111.378369]  __handle_irq_event_percpu+0x3c/0x340
> [  111.378374]  handle_irq_event_percpu+0x1b/0x50
> [  111.378378]  handle_irq_event+0x2f/0x50
> [  111.378381]  handle_fasteoi_irq+0x93/0x150
> [  111.378386]  handle_irq+0x11/0x20
> [  111.378390]  do_IRQ+0x5e/0x120
> [  111.378395]  common_interrupt+0xbb/0xbb
> [  111.378397]  
> 
> does tell us that acpi_os_execute() is called in irq context.

Well, right, thanks!

I overlooked this instance, so dropping the patch.

Thanks,
Rafael

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thursday, February 8, 2018 11:13:10 AM CET Chris Wilson wrote:
> Quoting Rafael J. Wysocki (2018-02-08 09:51:41)
> > On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> > > After checking all possible call chains to acpi_os_execute here,
> > > my tool finds that acpi_os_execute is never called in atomic context.
> > > And acpi_os_execute calls acpi_debugger_create_thread 
> > > which calls mutex_lock,
> > > thus it proves again that acpi_os_execute can 
> > > call functions which may sleep.
> > > Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> > > 
> > > This is found by a static analysis tool named DCNS written by myself.
> > > 
> > > Signed-off-by: Jia-Ju Bai 
> > > ---
> > >  drivers/acpi/osl.c |2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > > index 3bb46cb..8ee605e 100644
> > > --- a/drivers/acpi/osl.c
> > > +++ b/drivers/acpi/osl.c
> > > @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
> > >* having a static work_struct.
> > >*/
> > >  
> > > - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> > > + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
> > >   if (!dpc)
> > >   return AE_NO_MEMORY;
> > >  
> > > 
> > 
> > Applied, thanks!
> 
> Hmm, not this patch per se, but 
> 
> https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
> [  111.378236] BUG: sleeping function called from invalid context at 
> mm/slab.h:420
> [  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
> gem_exec_flush
> [  111.378275] INFO: lockdep is turned off.
> [  111.378277] irq event stamp: 0
> [  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
> (null)
> [  111.378286] hardirqs last disabled at (0): [] 
> copy_process.part.7+0x2f1/0x1db0
> [  111.378290] softirqs last  enabled at (0): [] 
> copy_process.part.7+0x2f1/0x1db0
> [  111.378292] softirqs last disabled at (0): [<  (null)>]   
> (null)
> [  111.378293] Preemption disabled at:
> [  111.378298] [] __mutex_lock+0x56/0x9b0
> [  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W  
>   4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
> [  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
> APLKRVPA.X64.0150.B11.1608081044 08/08/2016
> [  111.378314] Call Trace:
> [  111.378318]  
> [  111.378323]  dump_stack+0x5f/0x86
> [  111.378328]  ___might_sleep+0x1d9/0x240
> [  111.378334]  ? acpi_os_execute+0x2d/0x130
> [  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
> [  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
> [  111.378347]  acpi_os_execute+0x2d/0x130
> [  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
> [  111.378355]  acpi_ev_gpe_detect+0x156/0x195
> [  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
> [  111.378365]  acpi_irq+0xd/0x30
> [  111.378369]  __handle_irq_event_percpu+0x3c/0x340
> [  111.378374]  handle_irq_event_percpu+0x1b/0x50
> [  111.378378]  handle_irq_event+0x2f/0x50
> [  111.378381]  handle_fasteoi_irq+0x93/0x150
> [  111.378386]  handle_irq+0x11/0x20
> [  111.378390]  do_IRQ+0x5e/0x120
> [  111.378395]  common_interrupt+0xbb/0xbb
> [  111.378397]  
> 
> does tell us that acpi_os_execute() is called in irq context.

Well, right, thanks!

I overlooked this instance, so dropping the patch.

Thanks,
Rafael

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Chris Wilson 
Quoting Rafael J. Wysocki (2018-02-08 09:51:41)
> On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> > After checking all possible call chains to acpi_os_execute here,
> > my tool finds that acpi_os_execute is never called in atomic context.
> > And acpi_os_execute calls acpi_debugger_create_thread 
> > which calls mutex_lock,
> > thus it proves again that acpi_os_execute can 
> > call functions which may sleep.
> > Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> > 
> > This is found by a static analysis tool named DCNS written by myself.
> > 
> > Signed-off-by: Jia-Ju Bai 
> > ---
> >  drivers/acpi/osl.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 3bb46cb..8ee605e 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
> >* having a static work_struct.
> >*/
> >  
> > - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> > + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
> >   if (!dpc)
> >   return AE_NO_MEMORY;
> >  
> > 
> 
> Applied, thanks!

Hmm, not this patch per se, but 

https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
[  111.378236] BUG: sleeping function called from invalid context at 
mm/slab.h:420
[  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
gem_exec_flush
[  111.378275] INFO: lockdep is turned off.
[  111.378277] irq event stamp: 0
[  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
(null)
[  111.378286] hardirqs last disabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378290] softirqs last  enabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378292] softirqs last disabled at (0): [<  (null)>]   
(null)
[  111.378293] Preemption disabled at:
[  111.378298] [] __mutex_lock+0x56/0x9b0
[  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
[  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
APLKRVPA.X64.0150.B11.1608081044 08/08/2016
[  111.378314] Call Trace:
[  111.378318]  
[  111.378323]  dump_stack+0x5f/0x86
[  111.378328]  ___might_sleep+0x1d9/0x240
[  111.378334]  ? acpi_os_execute+0x2d/0x130
[  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
[  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
[  111.378347]  acpi_os_execute+0x2d/0x130
[  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
[  111.378355]  acpi_ev_gpe_detect+0x156/0x195
[  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
[  111.378365]  acpi_irq+0xd/0x30
[  111.378369]  __handle_irq_event_percpu+0x3c/0x340
[  111.378374]  handle_irq_event_percpu+0x1b/0x50
[  111.378378]  handle_irq_event+0x2f/0x50
[  111.378381]  handle_fasteoi_irq+0x93/0x150
[  111.378386]  handle_irq+0x11/0x20
[  111.378390]  do_IRQ+0x5e/0x120
[  111.378395]  common_interrupt+0xbb/0xbb
[  111.378397]  

does tell us that acpi_os_execute() is called in irq context.
-Chris

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Chris Wilson 
Quoting Rafael J. Wysocki (2018-02-08 09:51:41)
> On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> > After checking all possible call chains to acpi_os_execute here,
> > my tool finds that acpi_os_execute is never called in atomic context.
> > And acpi_os_execute calls acpi_debugger_create_thread 
> > which calls mutex_lock,
> > thus it proves again that acpi_os_execute can 
> > call functions which may sleep.
> > Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> > 
> > This is found by a static analysis tool named DCNS written by myself.
> > 
> > Signed-off-by: Jia-Ju Bai 
> > ---
> >  drivers/acpi/osl.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 3bb46cb..8ee605e 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
> >* having a static work_struct.
> >*/
> >  
> > - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> > + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
> >   if (!dpc)
> >   return AE_NO_MEMORY;
> >  
> > 
> 
> Applied, thanks!

Hmm, not this patch per se, but 

https://intel-gfx-ci.01.org/tree/next/next-20180207/fi-bxt-dsi/dmesg0.log
[  111.378236] BUG: sleeping function called from invalid context at 
mm/slab.h:420
[  111.378259] in_atomic(): 1, irqs_disabled(): 1, pid: 1701, name: 
gem_exec_flush
[  111.378275] INFO: lockdep is turned off.
[  111.378277] irq event stamp: 0
[  111.378280] hardirqs last  enabled at (0): [<  (null)>]   
(null)
[  111.378286] hardirqs last disabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378290] softirqs last  enabled at (0): [] 
copy_process.part.7+0x2f1/0x1db0
[  111.378292] softirqs last disabled at (0): [<  (null)>]   
(null)
[  111.378293] Preemption disabled at:
[  111.378298] [] __mutex_lock+0x56/0x9b0
[  111.378311] CPU: 1 PID: 1701 Comm: gem_exec_flush Tainted: G U  W
4.15.0-next-20180207-g5d1c98967100-next-20180207 #1
[  111.378313] Hardware name: Intel Corp. Broxton P/Apollolake RVP1A, BIOS 
APLKRVPA.X64.0150.B11.1608081044 08/08/2016
[  111.378314] Call Trace:
[  111.378318]  
[  111.378323]  dump_stack+0x5f/0x86
[  111.378328]  ___might_sleep+0x1d9/0x240
[  111.378334]  ? acpi_os_execute+0x2d/0x130
[  111.378338]  kmem_cache_alloc_trace+0x1ae/0x2b0
[  111.378344]  ? acpi_ev_asynch_enable_gpe+0x2d/0x2d
[  111.378347]  acpi_os_execute+0x2d/0x130
[  111.378351]  acpi_ev_gpe_dispatch+0xd7/0x120
[  111.378355]  acpi_ev_gpe_detect+0x156/0x195
[  111.378362]  acpi_ev_sci_xrupt_handler+0x16/0x28
[  111.378365]  acpi_irq+0xd/0x30
[  111.378369]  __handle_irq_event_percpu+0x3c/0x340
[  111.378374]  handle_irq_event_percpu+0x1b/0x50
[  111.378378]  handle_irq_event+0x2f/0x50
[  111.378381]  handle_fasteoi_irq+0x93/0x150
[  111.378386]  handle_irq+0x11/0x20
[  111.378390]  do_IRQ+0x5e/0x120
[  111.378395]  common_interrupt+0xbb/0xbb
[  111.378397]  

does tell us that acpi_os_execute() is called in irq context.
-Chris

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> After checking all possible call chains to acpi_os_execute here,
> my tool finds that acpi_os_execute is never called in atomic context.
> And acpi_os_execute calls acpi_debugger_create_thread 
> which calls mutex_lock,
> thus it proves again that acpi_os_execute can 
> call functions which may sleep.
> Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> 
> This is found by a static analysis tool named DCNS written by myself.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/acpi/osl.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb..8ee605e 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>* having a static work_struct.
>*/
>  
> - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
>   if (!dpc)
>   return AE_NO_MEMORY;
>  
> 

Applied, thanks!

Re: [PATCH] acpi: osl: Replace GFP_ATOMIC with GFP_KERNEL in acpi_os_execute

2018-02-08 Thread Rafael J. Wysocki 
On Thursday, January 25, 2018 11:13:41 AM CET Jia-Ju Bai wrote:
> After checking all possible call chains to acpi_os_execute here,
> my tool finds that acpi_os_execute is never called in atomic context.
> And acpi_os_execute calls acpi_debugger_create_thread 
> which calls mutex_lock,
> thus it proves again that acpi_os_execute can 
> call functions which may sleep.
> Thus GFP_ATOMIC is not necessary, and it can be replaced with GFP_KERNEL.
> 
> This is found by a static analysis tool named DCNS written by myself.
> 
> Signed-off-by: Jia-Ju Bai 
> ---
>  drivers/acpi/osl.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 3bb46cb..8ee605e 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -1066,7 +1066,7 @@ acpi_status acpi_os_execute(acpi_execute_type type,
>* having a static work_struct.
>*/
>  
> - dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_ATOMIC);
> + dpc = kzalloc(sizeof(struct acpi_os_dpc), GFP_KERNEL);
>   if (!dpc)
>   return AE_NO_MEMORY;
>  
> 

Applied, thanks!