* [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
@ 2018-03-18 14:53 Jia-Ju Bai
2018-03-18 22:40 ` Michael Kelley (EOSG)
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jia-Ju Bai @ 2018-03-18 14:53 UTC (permalink / raw)
To: kys, haiyangz, sthemmin, bhelgaas
Cc: devel, Jia-Ju Bai, linux-kernel, linux-pci
hv_pci_onchannelcallback() is not called in atomic context.
The call chain ending up at hv_pci_onchannelcallback() is:
[1] hv_pci_onchannelcallback() <- hv_pci_probe()
hv_pci_probe() is only set as ".probe" in hv_driver
structure "hv_pci_drv".
Despite never getting called from atomic context,
hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC,
which waits busily for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
to avoid busy waiting.
This is found by a static analysis tool named DCNS written by myself.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
drivers/pci/host/pci-hyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..c5c8a99 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void *context)
struct pci_dev_incoming *dev_message;
struct hv_pci_dev *hpdev;
- buffer = kmalloc(bufferlen, GFP_ATOMIC);
+ buffer = kmalloc(bufferlen, GFP_KERNEL);
if (!buffer)
return;
@@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void *context)
kfree(buffer);
/* Handle large packet */
bufferlen = bytes_recvd;
- buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
+ buffer = kmalloc(bytes_recvd, GFP_KERNEL);
if (!buffer)
return;
continue;
--
1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-18 14:53 [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback Jia-Ju Bai
@ 2018-03-18 22:40 ` Michael Kelley (EOSG)
2018-03-19 2:52 ` KY Srinivasan
2018-03-19 8:38 ` Dan Carpenter
2 siblings, 0 replies; 7+ messages in thread
From: Michael Kelley (EOSG) @ 2018-03-18 22:40 UTC (permalink / raw)
To: Jia-Ju Bai, KY Srinivasan, Haiyang Zhang, Stephen Hemminger, bhelgaas
Cc: devel, linux-kernel, linux-pci
> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org <linux-kernel-owner@vger.kernel.org> On Behalf
> Of Jia-Ju Bai
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Stephen
> Hemminger <sthemmin@microsoft.com>; bhelgaas@google.com
> Cc: devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org;
> Jia-Ju Bai <baijiaju1990@gmail.com>
> Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in
> hv_pci_onchannelcallback
>
> hv_pci_onchannelcallback() is not called in atomic context.
>
> The call chain ending up at hv_pci_onchannelcallback() is:
> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
> hv_pci_probe() is only set as ".probe" in hv_driver
> structure "hv_pci_drv".
>
> Despite never getting called from atomic context,
Not true. hv_pci_probe() registers hv_pci_onchannelcallback() as
a callback function that is invoked from the VMbus interrupt handler.
So GFP_ATOMIC is appropriate.
Michael
> hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
> to avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> drivers/pci/host/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..c5c8a99 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void *context)
> struct pci_dev_incoming *dev_message;
> struct hv_pci_dev *hpdev;
>
> - buffer = kmalloc(bufferlen, GFP_ATOMIC);
> + buffer = kmalloc(bufferlen, GFP_KERNEL);
> if (!buffer)
> return;
>
> @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void *context)
> kfree(buffer);
> /* Handle large packet */
> bufferlen = bytes_recvd;
> - buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
> + buffer = kmalloc(bytes_recvd, GFP_KERNEL);
> if (!buffer)
> return;
> continue;
> --
> 1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-18 14:53 [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback Jia-Ju Bai
2018-03-18 22:40 ` Michael Kelley (EOSG)
@ 2018-03-19 2:52 ` KY Srinivasan
2018-03-19 3:44 ` Jia-Ju Bai
2018-03-19 8:38 ` Dan Carpenter
2 siblings, 1 reply; 7+ messages in thread
From: KY Srinivasan @ 2018-03-19 2:52 UTC (permalink / raw)
To: Jia-Ju Bai, Haiyang Zhang, Stephen Hemminger, bhelgaas
Cc: devel, linux-kernel, linux-pci
> -----Original Message-----
> From: Jia-Ju Bai <baijiaju1990@gmail.com>
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; bhelgaas@google.com
> Cc: devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; Jia-Ju Bai <baijiaju1990@gmail.com>
> Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with
> GFP_KERNEL in hv_pci_onchannelcallback
>
> hv_pci_onchannelcallback() is not called in atomic context.
>
> The call chain ending up at hv_pci_onchannelcallback() is:
> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
> hv_pci_probe() is only set as ".probe" in hv_driver
> structure "hv_pci_drv".
This function is setup as the function to handle interrupts on the channel. Hence the
need for GFP_ATOMIC.
K. Y
>
> Despite never getting called from atomic context,
> hv_pci_onchannelcallback() calls kmalloc with GFP_ATOMIC,
> which waits busily for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL
> to avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> ---
> drivers/pci/host/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..c5c8a99 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1887,7 +1887,7 @@ static void hv_pci_onchannelcallback(void
> *context)
> struct pci_dev_incoming *dev_message;
> struct hv_pci_dev *hpdev;
>
> - buffer = kmalloc(bufferlen, GFP_ATOMIC);
> + buffer = kmalloc(bufferlen, GFP_KERNEL);
> if (!buffer)
> return;
>
> @@ -1899,7 +1899,7 @@ static void hv_pci_onchannelcallback(void
> *context)
> kfree(buffer);
> /* Handle large packet */
> bufferlen = bytes_recvd;
> - buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
> + buffer = kmalloc(bytes_recvd, GFP_KERNEL);
> if (!buffer)
> return;
> continue;
> --
> 1.9.1
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-19 2:52 ` KY Srinivasan
@ 2018-03-19 3:44 ` Jia-Ju Bai
0 siblings, 0 replies; 7+ messages in thread
From: Jia-Ju Bai @ 2018-03-19 3:44 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, bhelgaas
Cc: devel, linux-kernel, linux-pci
On 2018/3/19 10:52, KY Srinivasan wrote:
>
>> -----Original Message-----
>> From: Jia-Ju Bai <baijiaju1990@gmail.com>
>> Sent: Sunday, March 18, 2018 7:53 AM
>> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
>> <haiyangz@microsoft.com>; Stephen Hemminger
>> <sthemmin@microsoft.com>; bhelgaas@google.com
>> Cc: devel@linuxdriverproject.org; linux-pci@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Jia-Ju Bai <baijiaju1990@gmail.com>
>> Subject: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with
>> GFP_KERNEL in hv_pci_onchannelcallback
>>
>> hv_pci_onchannelcallback() is not called in atomic context.
>>
>> The call chain ending up at hv_pci_onchannelcallback() is:
>> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
>> hv_pci_probe() is only set as ".probe" in hv_driver
>> structure "hv_pci_drv".
> This function is setup as the function to handle interrupts on the channel. Hence the
> need for GFP_ATOMIC.
>
Oh, sorry for my incorrect patch.
Thanks for your reply :)
Best wishes,
Jia-Ju Bai
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-18 14:53 [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback Jia-Ju Bai
2018-03-18 22:40 ` Michael Kelley (EOSG)
2018-03-19 2:52 ` KY Srinivasan
@ 2018-03-19 8:38 ` Dan Carpenter
2018-03-19 8:53 ` Jia-Ju Bai
2 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2018-03-19 8:38 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: sthemmin, linux-pci, haiyangz, linux-kernel, bhelgaas, devel
On Sun, Mar 18, 2018 at 10:53:02PM +0800, Jia-Ju Bai wrote:
> hv_pci_onchannelcallback() is not called in atomic context.
>
> The call chain ending up at hv_pci_onchannelcallback() is:
> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
> hv_pci_probe() is only set as ".probe" in hv_driver
> structure "hv_pci_drv".
>
Your static analysis tool is faulty and apparently so is Smatch.
$ smdb function_ptrs hv_pci_onchannelcallback
Says it can't find a caller. When I look for function pointers I get:
$ smdb function_ptr hv_pci_onchannelcallback
hv_pci_onchannelcallback = 'hv_pci_onchannelcallback' , 'vmbus_open param 5' , '(struct vmbus_channel)->onchannel_callback' , '__read_once_size param 0'
Anyway the call tree is:
vmbus_chan_sched() <-- takes rcu_read_lock();
-> vmbus_channel_isr()
-> channel->onchannel_callback() -> which is hv_pci_onchannelcallback()
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-19 8:38 ` Dan Carpenter
@ 2018-03-19 8:53 ` Jia-Ju Bai
2018-03-19 10:29 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Jia-Ju Bai @ 2018-03-19 8:53 UTC (permalink / raw)
To: Dan Carpenter
Cc: sthemmin, linux-pci, haiyangz, linux-kernel, bhelgaas, devel
On 2018/3/19 16:38, Dan Carpenter wrote:
> On Sun, Mar 18, 2018 at 10:53:02PM +0800, Jia-Ju Bai wrote:
>> hv_pci_onchannelcallback() is not called in atomic context.
>>
>> The call chain ending up at hv_pci_onchannelcallback() is:
>> [1] hv_pci_onchannelcallback() <- hv_pci_probe()
>> hv_pci_probe() is only set as ".probe" in hv_driver
>> structure "hv_pci_drv".
>>
> Your static analysis tool is faulty and apparently so is Smatch.
>
> $ smdb function_ptrs hv_pci_onchannelcallback
>
> Says it can't find a caller. When I look for function pointers I get:
>
> $ smdb function_ptr hv_pci_onchannelcallback
> hv_pci_onchannelcallback = 'hv_pci_onchannelcallback' , 'vmbus_open param 5' , '(struct vmbus_channel)->onchannel_callback' , '__read_once_size param 0'
>
> Anyway the call tree is:
>
> vmbus_chan_sched() <-- takes rcu_read_lock();
> -> vmbus_channel_isr()
> -> channel->onchannel_callback() -> which is hv_pci_onchannelcallback(
Thanks for your reply :)
I admit my tool produces a false positive for this code...
Sorry for my incorrect patch.
Anyway, I find that function pointers are quite hard to analyze in the
Linux kernel code, because their usages are often flexible.
Have you found a good way to handle function pointers? Or can you
recommend some good tools to handle them?
Best wishes,
Jia-Ju Bai
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback
2018-03-19 8:53 ` Jia-Ju Bai
@ 2018-03-19 10:29 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2018-03-19 10:29 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: sthemmin, linux-pci, haiyangz, linux-kernel, bhelgaas, devel
Smatch tracks information about every function call. When a function
pointer is called it maybe looks something like this:
kernel/module.c | SYSC_delete_module | (struct module)->exit | INTERNAL | -1 | | void(*)()
So then we just have to know what functions are assigned to
module->exit.
I also filter based on the function signature "void(*)()" because
there are a couple places where we cut and pasted so the structs can
have the same name and function pointer name as a member but take
different arguments.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-19 10:29 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-18 14:53 [PATCH 1/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in hv_pci_onchannelcallback Jia-Ju Bai
2018-03-18 22:40 ` Michael Kelley (EOSG)
2018-03-19 2:52 ` KY Srinivasan
2018-03-19 3:44 ` Jia-Ju Bai
2018-03-19 8:38 ` Dan Carpenter
2018-03-19 8:53 ` Jia-Ju Bai
2018-03-19 10:29 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).