On Mon, May 16, 2022 at 04:58:44PM +0000, Von Dentz, Luiz wrote:
> Hi Stephen,
>
> Interesting, so we may want to stop using bitmap_from_u64 and replace with
> bitmap_from_arr32 given the former seems to expect at least 8 bytes:
Hi Luiz,
The problem is that br_params->flags and hdev->conn_flags are bitmaps
(declared with DECLARE_BITMAP), while cp->current_flags is declared
as u32.
Is it possible to declare cp->current_flags with DECLARE_BITMAP, or
declare local current_flags as unsigned long?
DECLARE_BITMAP(current_flags, __HCI_CONN_NUM_FLAGS) = {cp->current_flags};
If so, you can drop this conversion to and from fixed size arrays,
in the following code and use bitmap API more consistently.
For example the line
if ((supported_flags | current_flags) != supported_flags)
would turn to:
if (bitmap_subset(supported_flags, current_flags))
Alternatively, because __HCI_CONN_NUM_FLAGS == 2, and if you don't
expect adding 30+ new any flags soon, you can drop bitmap API here
and use hweight32/64 as appropriate.
Thanks,
Yury
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index 74937a834648..878be1cac5b7 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -4519,7 +4519,8 @@ static int set_device_flags(struct sock *sk, struct
> hci_dev *hdev, void *data,
> cp->addr.type);
>
> if (br_params) {
> - bitmap_from_u64(br_params->flags, current_flags);
> + bitmap_from_arr32(br_params->flags, ¤t_flags,
> + __HCI_CONN_NUM_FLAGS);
> status = MGMT_STATUS_SUCCESS;
> } else {
> bt_dev_warn(hdev, "No such BR/EDR device %pMR (0x%x)",
> @@ -4531,7 +4532,8 @@ static int set_device_flags(struct sock *sk, struct
> hci_dev *hdev, void *data,
> if (params) {
> DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
>
> - bitmap_from_u64(flags, current_flags);
> + bitmap_from_arr32(flags, ¤t_flags,
> + __HCI_CONN_NUM_FLAGS);
>
> /* Devices using RPAs can only be programmed in the
> * acceptlist LL Privacy has been enable otherwise they
> @@ -4546,7 +4548,8 @@ static int set_device_flags(struct sock *sk, struct
> hci_dev *hdev, void *data,
> goto unlock;
> }
>
> - bitmap_from_u64(params->flags, current_flags);
> + bitmap_from_arr32(params->flags, ¤t_flags,
> + __HCI_CONN_NUM_FLAGS);
> status = MGMT_STATUS_SUCCESS;
>
> /* Update passive scan if HCI_CONN_FLAG_DEVICE_PRIVACY
>
>
> ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> From: Stephen Rothwell
> Sent: Monday, May 16, 2022 12:57 AM
> To: Marcel Holtmann; Johan Hedberg; Yury Norov
> Cc: Von Dentz, Luiz; Linux Kernel Mailing List; Linux Next Mailing List
> Subject: linux-next: build warning after merge of the bluetooth tree
>
> Hi all,
>
> After merging the bluetooth tree, today's linux-next build (arm
> multi_v7_defconfig) produced this warning:
>
> In file included from include/linux/cpumask.h:12,
> from include/linux/mm_types_task.h:14,
> from include/linux/mm_types.h:5,
> from include/linux/buildid.h:5,
> from include/linux/module.h:14,
> from net/bluetooth/mgmt.c:27:
> In function 'bitmap_copy',
> inlined from 'bitmap_copy_clear_tail' at include/linux/bitmap.h:270:2,
> inlined from 'bitmap_from_u64' at include/linux/bitmap.h:622:2,
> inlined from 'set_device_flags' at net/bluetooth/mgmt.c:4534:4:
> include/linux/bitmap.h:261:9: warning: 'memcpy' forming offset [4, 7] is out of
> the bounds [0, 4] of object 'flags' with type 'long unsigned int[1]'
> [-Warray-bounds]
> 261 | memcpy(dst, src, len);
> | ^~~~~~~~~~~~~~~~~~~~~
> In file included from include/linux/kasan-checks.h:5,
> from include/asm-generic/rwonce.h:26,
> from ./arch/arm/include/generated/asm/rwonce.h:1,
> from include/linux/compiler.h:248,
> from include/linux/build_bug.h:5,
> from include/linux/container_of.h:5,
> from include/linux/list.h:5,
> from include/linux/module.h:12,
> from net/bluetooth/mgmt.c:27:
> net/bluetooth/mgmt.c: In function 'set_device_flags':
> net/bluetooth/mgmt.c:4532:40: note: 'flags' declared here
> 4532 | DECLARE_BITMAP(flags, __HCI_CONN_NUM_FLAGS);
> | ^~~~~
> include/linux/types.h:11:23: note: in definition of macro 'DECLARE_BITMAP'
> 11 | unsigned long name[BITS_TO_LONGS(bits)]
> | ^~~~
>
> Introduced by commit
>
> a9a347655d22 ("Bluetooth: MGMT: Add conditions for setting
> HCI_CONN_FLAG_REMOTE_WAKEUP")
>
> Bitmaps consist of unsigned longs (in this case 32 bits) ...
>
> (This warning only happens due to chnges in the bitmap tree.)
>
> --
> Cheers,
> Stephen Rothwell
