* [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv
@ 2018-01-26 9:37 Jia-Ju Bai
2018-01-27 17:48 ` Ed Cashin
0 siblings, 1 reply; 4+ messages in thread
From: Jia-Ju Bai @ 2018-01-26 9:37 UTC (permalink / raw)
To: ed.cashin; +Cc: linux-kernel, Jia-Ju Bai
After checking all possible call chains to aoenet_rcv(),
my tool finds that aoenet_rcv() is never called in atomic context,
namely never in an interrupt handler or holding a spinlock.
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 <baijiaju1990@gmail.com>
---
drivers/block/aoe/aoenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
index 63773a9..d5fff7a 100644
--- a/drivers/block/aoe/aoenet.c
+++ b/drivers/block/aoe/aoenet.c
@@ -138,7 +138,7 @@ static int __init aoe_iflist_setup(char *str)
if (dev_net(ifp) != &init_net)
goto exit;
- skb = skb_share_check(skb, GFP_ATOMIC);
+ skb = skb_share_check(skb, GFP_KERNEL);
if (skb == NULL)
return 0;
if (!is_aoe_netif(ifp))
--
1.7.9.5
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv
2018-01-26 9:37 [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv Jia-Ju Bai
@ 2018-01-27 17:48 ` Ed Cashin
2018-01-28 2:12 ` Jia-Ju Bai
0 siblings, 1 reply; 4+ messages in thread
From: Ed Cashin @ 2018-01-27 17:48 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: ed.cashin, linux-kernel
If the tool cannot tell whether the protected state is manipulated by *another* piece of code called in atomic context, then it's insufficient.
> On Jan 26, 2018, at 4:37 AM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
> After checking all possible call chains to aoenet_rcv(),
> my tool finds that aoenet_rcv() is never called in atomic context,
> namely never in an interrupt handler or holding a spinlock.
> 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 <baijiaju1990@gmail.com>
> ---
> drivers/block/aoe/aoenet.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
> index 63773a9..d5fff7a 100644
> --- a/drivers/block/aoe/aoenet.c
> +++ b/drivers/block/aoe/aoenet.c
> @@ -138,7 +138,7 @@ static int __init aoe_iflist_setup(char *str)
> if (dev_net(ifp) != &init_net)
> goto exit;
>
> - skb = skb_share_check(skb, GFP_ATOMIC);
> + skb = skb_share_check(skb, GFP_KERNEL);
> if (skb == NULL)
> return 0;
> if (!is_aoe_netif(ifp))
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv
2018-01-27 17:48 ` Ed Cashin
@ 2018-01-28 2:12 ` Jia-Ju Bai
2018-01-28 15:40 ` Ed Cashin
0 siblings, 1 reply; 4+ messages in thread
From: Jia-Ju Bai @ 2018-01-28 2:12 UTC (permalink / raw)
To: Ed Cashin; +Cc: ed.cashin, linux-kernel
On 2018/1/28 1:48, Ed Cashin wrote:
> If the tool cannot tell whether the protected state is manipulated by *another* piece of code called in atomic context, then it's insufficient.
>
>> On Jan 26, 2018, at 4:37 AM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>> After checking all possible call chains to aoenet_rcv(),
>> my tool finds that aoenet_rcv() is never called in atomic context,
>> namely never in an interrupt handler or holding a spinlock.
>> 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 <baijiaju1990@gmail.com>
>> ---
>> drivers/block/aoe/aoenet.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
>> index 63773a9..d5fff7a 100644
>> --- a/drivers/block/aoe/aoenet.c
>> +++ b/drivers/block/aoe/aoenet.c
>> @@ -138,7 +138,7 @@ static int __init aoe_iflist_setup(char *str)
>> if (dev_net(ifp) != &init_net)
>> goto exit;
>>
>> - skb = skb_share_check(skb, GFP_ATOMIC);
>> + skb = skb_share_check(skb, GFP_KERNEL);
>> if (skb == NULL)
>> return 0;
>> if (!is_aoe_netif(ifp))
>> --
>> 1.7.9.5
>>
>>
Sorry, I find my report is false positive after I manually check the code.
aoenet_rcv() is used as function pointer via "->func", and it is called
in dev_queue_xmit_nit() in net/core/dev.c.
dev_queue_xmit_nit() calls a rcu_read_lock() before it calls
pt_prev->func().
Thus it is right to use GFP_ATOMIC in aoenet_rcv().
Sorry again for my incorrect report...
Thanks,
Jia-Ju Bai
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv
2018-01-28 2:12 ` Jia-Ju Bai
@ 2018-01-28 15:40 ` Ed Cashin
0 siblings, 0 replies; 4+ messages in thread
From: Ed Cashin @ 2018-01-28 15:40 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: ed.cashin, linux-kernel
Good luck in your efforts, and thanks for your work on static analysis.
> On Jan 27, 2018, at 9:12 PM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
>
>
>> On 2018/1/28 1:48, Ed Cashin wrote:
>> If the tool cannot tell whether the protected state is manipulated by *another* piece of code called in atomic context, then it's insufficient.
>>
>>> On Jan 26, 2018, at 4:37 AM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>>
>>> After checking all possible call chains to aoenet_rcv(),
>>> my tool finds that aoenet_rcv() is never called in atomic context,
>>> namely never in an interrupt handler or holding a spinlock.
>>> 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 <baijiaju1990@gmail.com>
>>> ---
>>> drivers/block/aoe/aoenet.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/block/aoe/aoenet.c b/drivers/block/aoe/aoenet.c
>>> index 63773a9..d5fff7a 100644
>>> --- a/drivers/block/aoe/aoenet.c
>>> +++ b/drivers/block/aoe/aoenet.c
>>> @@ -138,7 +138,7 @@ static int __init aoe_iflist_setup(char *str)
>>> if (dev_net(ifp) != &init_net)
>>> goto exit;
>>>
>>> - skb = skb_share_check(skb, GFP_ATOMIC);
>>> + skb = skb_share_check(skb, GFP_KERNEL);
>>> if (skb == NULL)
>>> return 0;
>>> if (!is_aoe_netif(ifp))
>>> --
>>> 1.7.9.5
>>>
>>>
>
> Sorry, I find my report is false positive after I manually check the code.
> aoenet_rcv() is used as function pointer via "->func", and it is called in dev_queue_xmit_nit() in net/core/dev.c.
> dev_queue_xmit_nit() calls a rcu_read_lock() before it calls pt_prev->func().
> Thus it is right to use GFP_ATOMIC in aoenet_rcv().
> Sorry again for my incorrect report...
>
> Thanks,
> Jia-Ju Bai
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-28 15:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-26 9:37 [PATCH] block: aoenet: Replace GFP_ATOMIC with GFP_KERNEL in aoenet_rcv Jia-Ju Bai
2018-01-27 17:48 ` Ed Cashin
2018-01-28 2:12 ` Jia-Ju Bai
2018-01-28 15:40 ` Ed Cashin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®