2018-04-11 01:34:43

by Jia-Ju Bai

[permalink] [raw]
Subject: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

stir421x_fw_upload() is never called in atomic context.

The call chain ending up at stir421x_fw_upload() is:
[1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()

irda_usb_probe() is set as ".probe" in struct usb_driver.
This function is not called in atomic context.

Despite never getting called from atomic context, stir421x_fw_upload()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() to
avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <[email protected]>
---
drivers/staging/irda/drivers/irda-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..c6c8c2c 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -1050,7 +1050,7 @@ static int stir421x_fw_upload(struct irda_usb_cb *self,
if (ret < 0)
break;

- mdelay(10);
+ usleep_range(10000, 11000);
}

kfree(patch_block);
--
1.9.1



2018-04-11 07:11:20

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
>
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
>
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() to
> avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <[email protected]>

This all looks good (also note the call to usb_control_msg(), which may
sleep, just above the mdelay()).

Reviewed-by: Johan Hovold <[email protected]>

Thanks,
Johan

2018-04-11 07:19:01

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
>
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
>
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() to
> avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <[email protected]>
> ---
> drivers/staging/irda/drivers/irda-usb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Please, at the very least, work off of Linus's tree. There is no
drivers/staging/irda/ anymore :)

thanks,

greg k-h

2018-04-11 07:21:27

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload



On 2018/4/11 14:41, Greg KH wrote:
> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>> stir421x_fw_upload() is never called in atomic context.
>>
>> The call chain ending up at stir421x_fw_upload() is:
>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>
>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>> This function is not called in atomic context.
>>
>> Despite never getting called from atomic context, stir421x_fw_upload()
>> calls mdelay() to busily wait.
>> This is not necessary and can be replaced with usleep_range() to
>> avoid busy waiting.
>>
>> This is found by a static analysis tool named DCNS written by myself.
>> And I also manually check it.
>>
>> Signed-off-by: Jia-Ju Bai <[email protected]>
>> ---
>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> Please, at the very least, work off of Linus's tree. There is no
> drivers/staging/irda/ anymore :)
>

Okay, sorry.
Could you please recommend me a right tree or its git address?


Best wishes,
Jia-Ju Bai

2018-04-11 08:07:01

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>
>
> On 2018/4/11 14:41, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > stir421x_fw_upload() is never called in atomic context.
> > >
> > > The call chain ending up at stir421x_fw_upload() is:
> > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > >
> > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > This function is not called in atomic context.
> > >
> > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > calls mdelay() to busily wait.
> > > This is not necessary and can be replaced with usleep_range() to
> > > avoid busy waiting.
> > >
> > > This is found by a static analysis tool named DCNS written by myself.
> > > And I also manually check it.
> > >
> > > Signed-off-by: Jia-Ju Bai <[email protected]>
> > > ---
> > > drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > Please, at the very least, work off of Linus's tree. There is no
> > drivers/staging/irda/ anymore :)
> >
>
> Okay, sorry.
> Could you please recommend me a right tree or its git address?

Have you looked in the MAINTAINERS file? Worst case, always use
linux-next.

greg k-h

2018-04-11 08:15:56

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload



On 2018/4/11 16:03, Greg KH wrote:
> On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>>
>> On 2018/4/11 14:41, Greg KH wrote:
>>> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>>>> stir421x_fw_upload() is never called in atomic context.
>>>>
>>>> The call chain ending up at stir421x_fw_upload() is:
>>>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>>>
>>>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>>>> This function is not called in atomic context.
>>>>
>>>> Despite never getting called from atomic context, stir421x_fw_upload()
>>>> calls mdelay() to busily wait.
>>>> This is not necessary and can be replaced with usleep_range() to
>>>> avoid busy waiting.
>>>>
>>>> This is found by a static analysis tool named DCNS written by myself.
>>>> And I also manually check it.
>>>>
>>>> Signed-off-by: Jia-Ju Bai <[email protected]>
>>>> ---
>>>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> Please, at the very least, work off of Linus's tree. There is no
>>> drivers/staging/irda/ anymore :)
>>>
>> Okay, sorry.
>> Could you please recommend me a right tree or its git address?
> Have you looked in the MAINTAINERS file? Worst case, always use
> linux-next.
>
> greg k-h

Oh, sorry, I did notice the git tree in the MAINTAINERS file.
I always used linux-stable.
Thanks for telling me this :)


Best wishes,
Jia-Ju Bai

2018-04-11 08:20:52

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
>
>
> On 2018/4/11 16:03, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> > >
> > > On 2018/4/11 14:41, Greg KH wrote:
> > > > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > > > stir421x_fw_upload() is never called in atomic context.
> > > > >
> > > > > The call chain ending up at stir421x_fw_upload() is:
> > > > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > > > >
> > > > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > > > This function is not called in atomic context.
> > > > >
> > > > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > > > calls mdelay() to busily wait.
> > > > > This is not necessary and can be replaced with usleep_range() to
> > > > > avoid busy waiting.
> > > > >
> > > > > This is found by a static analysis tool named DCNS written by myself.
> > > > > And I also manually check it.
> > > > >
> > > > > Signed-off-by: Jia-Ju Bai <[email protected]>
> > > > > ---
> > > > > drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > Please, at the very least, work off of Linus's tree. There is no
> > > > drivers/staging/irda/ anymore :)
> > > >
> > > Okay, sorry.
> > > Could you please recommend me a right tree or its git address?
> > Have you looked in the MAINTAINERS file? Worst case, always use
> > linux-next.
> >
> > greg k-h
>
> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
> I always used linux-stable.

linux-stable is almost never the tree to use as it is almost always
12000 patches behind what is in Linus's tree and about 20000 changes
behind linux-next.

thanks,

greg k-h

2018-04-11 08:25:00

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload



On 2018/4/11 16:17, Greg KH wrote:
> On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
>>
>> On 2018/4/11 16:03, Greg KH wrote:
>>> On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>>>> On 2018/4/11 14:41, Greg KH wrote:
>>>>> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>>>>>> stir421x_fw_upload() is never called in atomic context.
>>>>>>
>>>>>> The call chain ending up at stir421x_fw_upload() is:
>>>>>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>>>>>
>>>>>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>>>>>> This function is not called in atomic context.
>>>>>>
>>>>>> Despite never getting called from atomic context, stir421x_fw_upload()
>>>>>> calls mdelay() to busily wait.
>>>>>> This is not necessary and can be replaced with usleep_range() to
>>>>>> avoid busy waiting.
>>>>>>
>>>>>> This is found by a static analysis tool named DCNS written by myself.
>>>>>> And I also manually check it.
>>>>>>
>>>>>> Signed-off-by: Jia-Ju Bai <[email protected]>
>>>>>> ---
>>>>>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> Please, at the very least, work off of Linus's tree. There is no
>>>>> drivers/staging/irda/ anymore :)
>>>>>
>>>> Okay, sorry.
>>>> Could you please recommend me a right tree or its git address?
>>> Have you looked in the MAINTAINERS file? Worst case, always use
>>> linux-next.
>>>
>>> greg k-h
>> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
>> I always used linux-stable.
> linux-stable is almost never the tree to use as it is almost always
> 12000 patches behind what is in Linus's tree and about 20000 changes
> behind linux-next.
>

Okay, I now know why many of my patches were not replied.
I should choose correct tree in the MAINTAINERS file.
Thanks :)


Best wishes,
Jia-Ju Bai

2018-04-11 14:29:39

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload

From: Jia-Ju Bai <[email protected]>
Date: Wed, 11 Apr 2018 16:20:22 +0800

> Okay, I now know why many of my patches were not replied.

Many of your patches are not responded to because you handle patch
feedback poorly sometimes.

Also, all of your networking submissions have been dropped because
the net-next tree is closed.


2018-04-11 14:34:33

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload



On 2018/4/11 22:26, David Miller wrote:
> From: Jia-Ju Bai <[email protected]>
> Date: Wed, 11 Apr 2018 16:20:22 +0800
>
>> Okay, I now know why many of my patches were not replied.
> Many of your patches are not responded to because you handle patch
> feedback poorly sometimes.

Okay, thanks for pointing it out.
I will handle patch feedback much more carefully.

> Also, all of your networking submissions have been dropped because
> the net-next tree is closed.
>

Okay, I will choose the proper tree to submit.


Best wishes,
Jia-Ju Bai