diff mbox series

[next] Bluetooth: use bitmap_empty to check if a bitmap has any bits set

Message ID 20211007111713.12207-1-colin.king@canonical.com (mailing list archive)
State Deferred
Headers show
Series [next] Bluetooth: use bitmap_empty to check if a bitmap has any bits set | expand

Checks

Context Check Description
tedd_an/checkpatch success Checkpatch PASS
tedd_an/gitlint success Gitlint PASS
tedd_an/buildkernel success Build Kernel PASS
tedd_an/testrunnersetup success Test Runner Setup PASS
tedd_an/testrunnerl2cap-tester success Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerbnep-tester success Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnermgmt-tester success Total: 463, Passed: 463 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnerrfcomm-tester success Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersco-tester success Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunnersmp-tester success Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
tedd_an/testrunneruserchan-tester success Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0

Commit Message

Colin King Oct. 7, 2021, 11:17 a.m. UTC 
From: Colin Ian King <colin.king@canonical.com>

The check to see if any tasks are left checks if bitmap array is zero
rather than using the appropriate bitmap helper functions to check the
bits in the array. Fix this by using bitmap_empty on the bitmap.

Addresses-Coverity: (" Array compared against 0")
Fixes: 912730b52552 ("Bluetooth: Fix wake up suspend_wait_q prematurely")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/bluetooth/hci_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

bluez.test.bot@gmail.com Oct. 7, 2021, 12:05 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=559055

---Test result---

Test Summary:
CheckPatch                    PASS      0.90 seconds
GitLint                       PASS      0.31 seconds
BuildKernel                   PASS      682.76 seconds
TestRunner: Setup             PASS      501.66 seconds
TestRunner: l2cap-tester      PASS      10.79 seconds
TestRunner: bnep-tester       PASS      5.66 seconds
TestRunner: mgmt-tester       PASS      95.25 seconds
TestRunner: rfcomm-tester     PASS      6.92 seconds
TestRunner: sco-tester        PASS      7.09 seconds
TestRunner: smp-tester        PASS      7.05 seconds
TestRunner: userchan-tester   PASS      5.88 seconds



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Oct. 7, 2021, 3:34 p.m. UTC | #2 
Hi Colin,

On Thu, Oct 7, 2021 at 4:17 AM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> The check to see if any tasks are left checks if bitmap array is zero
> rather than using the appropriate bitmap helper functions to check the
> bits in the array. Fix this by using bitmap_empty on the bitmap.
>
> Addresses-Coverity: (" Array compared against 0")
> Fixes: 912730b52552 ("Bluetooth: Fix wake up suspend_wait_q prematurely")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/bluetooth/hci_request.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> index 209f4fe17237..bad3b9c895ba 100644
> --- a/net/bluetooth/hci_request.c
> +++ b/net/bluetooth/hci_request.c
> @@ -1108,7 +1108,7 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
>         clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
>
>         /* Wake up only if there are no tasks left */
> -       if (!hdev->suspend_tasks)
> +       if (!bitmap_empty(hdev->suspend_tasks, __SUSPEND_NUM_TASKS))
>                 wake_up(&hdev->suspend_wait_q);
>  }
>
> --
> 2.32.0

I was going to revert this change since it appears wake_up does
actually check the wake condition there is no premature wake up after
all.
Colin King Oct. 7, 2021, 3:39 p.m. UTC | #3 
On 07/10/2021 16:34, Luiz Augusto von Dentz wrote:
> Hi Colin,
> 
> On Thu, Oct 7, 2021 at 4:17 AM Colin King <colin.king@canonical.com> wrote:
>>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The check to see if any tasks are left checks if bitmap array is zero
>> rather than using the appropriate bitmap helper functions to check the
>> bits in the array. Fix this by using bitmap_empty on the bitmap.
>>
>> Addresses-Coverity: (" Array compared against 0")
>> Fixes: 912730b52552 ("Bluetooth: Fix wake up suspend_wait_q prematurely")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>   net/bluetooth/hci_request.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
>> index 209f4fe17237..bad3b9c895ba 100644
>> --- a/net/bluetooth/hci_request.c
>> +++ b/net/bluetooth/hci_request.c
>> @@ -1108,7 +1108,7 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
>>          clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
>>
>>          /* Wake up only if there are no tasks left */
>> -       if (!hdev->suspend_tasks)
>> +       if (!bitmap_empty(hdev->suspend_tasks, __SUSPEND_NUM_TASKS))
>>                  wake_up(&hdev->suspend_wait_q);
>>   }
>>
>> --
>> 2.32.0
> 
> I was going to revert this change since it appears wake_up does
> actually check the wake condition there is no premature wake up after
> all.
> 
OK, makes sense.

Colin
Marcel Holtmann Oct. 7, 2021, 3:47 p.m. UTC | #4 
Hi Luiz,

>> The check to see if any tasks are left checks if bitmap array is zero
>> rather than using the appropriate bitmap helper functions to check the
>> bits in the array. Fix this by using bitmap_empty on the bitmap.
>> 
>> Addresses-Coverity: (" Array compared against 0")
>> Fixes: 912730b52552 ("Bluetooth: Fix wake up suspend_wait_q prematurely")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> net/bluetooth/hci_request.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
>> index 209f4fe17237..bad3b9c895ba 100644
>> --- a/net/bluetooth/hci_request.c
>> +++ b/net/bluetooth/hci_request.c
>> @@ -1108,7 +1108,7 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
>>        clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
>> 
>>        /* Wake up only if there are no tasks left */
>> -       if (!hdev->suspend_tasks)
>> +       if (!bitmap_empty(hdev->suspend_tasks, __SUSPEND_NUM_TASKS))
>>                wake_up(&hdev->suspend_wait_q);
>> }
>> 
>> --
>> 2.32.0
> 
> I was going to revert this change since it appears wake_up does
> actually check the wake condition there is no premature wake up after
> all.

so should I take the patch "Fix wake up suspend_wait_q prematurely” completely out?

Regards

Marcel
Luiz Augusto von Dentz Oct. 7, 2021, 3:53 p.m. UTC | #5 
Hi Marcel,

On Thu, Oct 7, 2021 at 8:47 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> >> The check to see if any tasks are left checks if bitmap array is zero
> >> rather than using the appropriate bitmap helper functions to check the
> >> bits in the array. Fix this by using bitmap_empty on the bitmap.
> >>
> >> Addresses-Coverity: (" Array compared against 0")
> >> Fixes: 912730b52552 ("Bluetooth: Fix wake up suspend_wait_q prematurely")
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >> net/bluetooth/hci_request.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
> >> index 209f4fe17237..bad3b9c895ba 100644
> >> --- a/net/bluetooth/hci_request.c
> >> +++ b/net/bluetooth/hci_request.c
> >> @@ -1108,7 +1108,7 @@ static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
> >>        clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
> >>
> >>        /* Wake up only if there are no tasks left */
> >> -       if (!hdev->suspend_tasks)
> >> +       if (!bitmap_empty(hdev->suspend_tasks, __SUSPEND_NUM_TASKS))
> >>                wake_up(&hdev->suspend_wait_q);
> >> }
> >>
> >> --
> >> 2.32.0
> >
> > I was going to revert this change since it appears wake_up does
> > actually check the wake condition there is no premature wake up after
> > all.
>
> so should I take the patch "Fix wake up suspend_wait_q prematurely” completely out?

Yes, please take it out.
diff mbox series

Patch

diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index 209f4fe17237..bad3b9c895ba 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -1108,7 +1108,7 @@  static void suspend_req_complete(struct hci_dev *hdev, u8 status, u16 opcode)
 	clear_bit(SUSPEND_SET_ADV_FILTER, hdev->suspend_tasks);
 
 	/* Wake up only if there are no tasks left */
-	if (!hdev->suspend_tasks)
+	if (!bitmap_empty(hdev->suspend_tasks, __SUSPEND_NUM_TASKS))
 		wake_up(&hdev->suspend_wait_q);
 }