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

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

ma, 2025-07-14 kello 19:09 +0200, Christian Eggers kirjoitti:

Quoted 11 lines

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.

Quoted 6 lines

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)

Quoted 17 lines

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.

Quoted 6 lines

Are there any alternatives? Anything I missed?

regards,
Christian

--
Pauli Virtanen