struct hci_dev::quirks is running out of bits on 32-bit platforms

struct hci_dev::quirks is running out of bits on 32-bit platforms

[Christian Eggers]

I just tried to introduce another quirk for Realtek Bluetooth controllers 
when I recognized that the underlying data type (unsigned long) has already
run out available bits on system where sizeof(unsigned long) == 4.
The number of entries in the (anonymous) quirks enum has already reached 34 
in the latest kernels.

My first temptation was to simply change the data type to something like __u64,
but this is not as easy as it seems. The test_bit() macro used almost everywhere
for assigning quirks is guaranteed to be atomic and my platform (ARMv7) seems
not to have support for atomic operations on __u64.

I mainly see two options:

1. Introducing a 'quirks2' member (bad)

This obviously would work, but requires another enum and will (I think)
introduce stupid bugs if the wrong quirks member is exercised.

2. Switch to using __64 with non atomic operations

About 99% of write accesses to the quirks member happen from probe() or
setup() routines which should (I hope) not allow simultaneous access from other
contexts. I found 2 exceptions (as of linux-6.12):

a. btusb_setup_qca() is called from 'struct hci_dev::open()' (maybe uncritical).
b. Two quirks (strict_duplicate_filter, simultaneous_discovery) can be toggled 
   via debugfs.

So it looks like using non atomic operations can also introduce trouble if
not well reviewed. But as the 'strict_duplicate_filter' and 
'simultaneous_discovery' quirks are only used at very few locations, maybe
these should be moved to a new member for "atomic quirks", allowing to
convert the remaining ones to non atomic.


Are there any alternatives? Anything I missed?

regards,
Christian

Re: struct hci_dev::quirks is running out of bits on 32-bit platforms

[Pauli Virtanen]

ma, 2025-07-14 kello 19:09 +0200, Christian Eggers kirjoitti:
> I just tried to introduce another quirk for Realtek Bluetooth controllers 
> when I recognized that the underlying data type (unsigned long) has already
> run out available bits on system where sizeof(unsigned long) == 4.
> The number of entries in the (anonymous) quirks enum has already reached 34 
> in the latest kernels.
> 
> My first temptation was to simply change the data type to something like __u64,
> but this is not as easy as it seems. The test_bit() macro used almost everywhere
> for assigning quirks is guaranteed to be atomic and my platform (ARMv7) seems
> not to have support for atomic operations on __u64.
> 
> I mainly see two options:
> 
> 1. Introducing a 'quirks2' member (bad)
> 
> This obviously would work, but requires another enum and will (I think)
> introduce stupid bugs if the wrong quirks member is exercised.

The pattern used for hci_dev::dev_flags is

	struct hci_dev {
		DECLARE_BITMAP(quirk_flags, __HCI_NUM_QUIRKS);
		...
	}
	#define hci_set_quirk(hdev, nr) set_bit((nr), (hdev)->quirk_flags)
	#define hci_clear_quirk(hdev, nr) clear_bit((nr), (hdev)->quirk_flags)
	#define hci_test_quirk(hdev, nr) test_bit((nr), (hdev)->quirk_flags)

> 2. Switch to using __64 with non atomic operations
> 
> About 99% of write accesses to the quirks member happen from probe() or
> setup() routines which should (I hope) not allow simultaneous access from other
> contexts. I found 2 exceptions (as of linux-6.12):
> 
> a. btusb_setup_qca() is called from 'struct hci_dev::open()' (maybe uncritical).
> b. Two quirks (strict_duplicate_filter, simultaneous_discovery) can be toggled 
>    via debugfs.
> 
> So it looks like using non atomic operations can also introduce trouble if
> not well reviewed. But as the 'strict_duplicate_filter' and 
> 'simultaneous_discovery' quirks are only used at very few locations, maybe
> these should be moved to a new member for "atomic quirks", allowing to
> convert the remaining ones to non atomic.
> 
> 
> Are there any alternatives? Anything I missed?
> 
> regards,
> Christian
> 
> 

-- 
Pauli Virtanen

Re: struct hci_dev::quirks is running out of bits on 32-bit platforms

[Luiz Augusto von Dentz]

Hi Pauli, Christian,

On Mon, Jul 14, 2025 at 2:03 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> ma, 2025-07-14 kello 19:09 +0200, Christian Eggers kirjoitti:
> > I just tried to introduce another quirk for Realtek Bluetooth controllers
> > when I recognized that the underlying data type (unsigned long) has already
> > run out available bits on system where sizeof(unsigned long) == 4.
> > The number of entries in the (anonymous) quirks enum has already reached 34
> > in the latest kernels.
> >
> > My first temptation was to simply change the data type to something like __u64,
> > but this is not as easy as it seems. The test_bit() macro used almost everywhere
> > for assigning quirks is guaranteed to be atomic and my platform (ARMv7) seems
> > not to have support for atomic operations on __u64.
> >
> > I mainly see two options:
> >
> > 1. Introducing a 'quirks2' member (bad)
> >
> > This obviously would work, but requires another enum and will (I think)
> > introduce stupid bugs if the wrong quirks member is exercised.
>
> The pattern used for hci_dev::dev_flags is
>
>         struct hci_dev {
>                 DECLARE_BITMAP(quirk_flags, __HCI_NUM_QUIRKS);
>                 ...
>         }
>         #define hci_set_quirk(hdev, nr) set_bit((nr), (hdev)->quirk_flags)
>         #define hci_clear_quirk(hdev, nr) clear_bit((nr), (hdev)->quirk_flags)
>         #define hci_test_quirk(hdev, nr) test_bit((nr), (hdev)->quirk_flags)

+1, that is probably the best option if we don't want to do something
bluetooth specific.

> > 2. Switch to using __64 with non atomic operations
> >
> > About 99% of write accesses to the quirks member happen from probe() or
> > setup() routines which should (I hope) not allow simultaneous access from other
> > contexts. I found 2 exceptions (as of linux-6.12):
> >
> > a. btusb_setup_qca() is called from 'struct hci_dev::open()' (maybe uncritical).
> > b. Two quirks (strict_duplicate_filter, simultaneous_discovery) can be toggled
> >    via debugfs.
> >
> > So it looks like using non atomic operations can also introduce trouble if
> > not well reviewed. But as the 'strict_duplicate_filter' and
> > 'simultaneous_discovery' quirks are only used at very few locations, maybe
> > these should be moved to a new member for "atomic quirks", allowing to
> > convert the remaining ones to non atomic.
> >
> >
> > Are there any alternatives? Anything I missed?
> >
> > regards,
> > Christian
> >
> >
>
> --
> Pauli Virtanen
>


-- 
Luiz Augusto von Dentz