2018-03-18 14:55:13

by Jia-Ju Bai

Subject: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

new_pcichild_device() is not called in atomic context.

The call chain ending up at new_pcichild_device() is:
[1] new_pcichild_device() <- pci_devices_present_work()
pci_devices_present_work() is only set in INIT_WORK().

Despite never getting called from atomic context,
new_pcichild_device() calls kzalloc 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 <[email protected]>
---
drivers/pci/host/pci-hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0fe3ea1..289e31d 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1500,7 +1500,7 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
unsigned long flags;
int ret;

- hpdev = kzalloc(sizeof(*hpdev), GFP_ATOMIC);
+ hpdev = kzalloc(sizeof(*hpdev), GFP_KERNEL);
if (!hpdev)
return NULL;

--
1.9.1

2018-03-18 22:43:51

by Michael Kelley (EOSG)

Subject: RE: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

> -----Original Message-----
> From: [email protected] <[email protected]> On Behalf
> Of Jia-Ju Bai
> Sent: Sunday, March 18, 2018 7:53 AM
> To: KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; Stephen
> Hemminger <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> Jia-Ju Bai <[email protected]>
> Subject: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in
> new_pcichild_device
>
> new_pcichild_device() is not called in atomic context.
>
> The call chain ending up at new_pcichild_device() is:
> [1] new_pcichild_device() <- pci_devices_present_work()
> pci_devices_present_work() is only set in INIT_WORK().
>
> Despite never getting called from atomic context,
> new_pcichild_device() calls kzalloc 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 <[email protected]>

Reviewed-by: Michael Kelley <[email protected]>

> ---
> drivers/pci/host/pci-hyperv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..289e31d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1500,7 +1500,7 @@ static struct hv_pci_dev *new_pcichild_device(struct
> hv_pcibus_device *hbus,
> unsigned long flags;
> int ret;
>
> - hpdev = kzalloc(sizeof(*hpdev), GFP_ATOMIC);
> + hpdev = kzalloc(sizeof(*hpdev), GFP_KERNEL);
> if (!hpdev)
> return NULL;
>
> --
> 1.9.1

2018-03-19 09:04:51

by Dan Carpenter

Subject: Re: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

Looks ok.

regards,
dan carpenter

2018-06-29 18:29:41

by Lorenzo Pieralisi

Subject: Re: [PATCH 2/2] pci: host: pci-hyperv: Replace GFP_ATOMIC with GFP_KERNEL in new_pcichild_device

On Sun, Mar 18, 2018 at 10:53:28PM +0800, Jia-Ju Bai wrote:
> new_pcichild_device() is not called in atomic context.
>
> The call chain ending up at new_pcichild_device() is:
> [1] new_pcichild_device() <- pci_devices_present_work()
> pci_devices_present_work() is only set in INIT_WORK().
>
> Despite never getting called from atomic context,
> new_pcichild_device() calls kzalloc 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 <[email protected]>
> ---
> drivers/pci/host/pci-hyperv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Applied to pci/hv for v4.19, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 0fe3ea1..289e31d 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1500,7 +1500,7 @@ static struct hv_pci_dev *new_pcichild_device(struct hv_pcibus_device *hbus,
> unsigned long flags;
> int ret;
>
> - hpdev = kzalloc(sizeof(*hpdev), GFP_ATOMIC);
> + hpdev = kzalloc(sizeof(*hpdev), GFP_KERNEL);
> if (!hpdev)
> return NULL;
>
> --
> 1.9.1
>