Why is multicore_doorbell_claim_unused(0x3) claiming two doorbell bits on every successful call? #2804

New issue

Closed

opened 2026-01-23 17:52:40 -06:00 by akohlsmith · 1 comment

akohlsmith commented

2026-01-23 17:52:40 -06:00

(Migrated from github.com)

int main(void)
{
    int a = multicore_doorbell_claim_unused(0x3, false);
    int b = multicore_doorbell_claim_unused(0x3, false);
    int c = multicore_doorbell_claim_unused(0x3, false);
    int d = multicore_doorbell_claim_unused(0x3, false);
    int e = multicore_doorbell_claim_unused(0x3, false);

    printf("a=%d, b=%d, c=%d, d=%d, e=%d\n", a, b, c, d, e);
    return 0;
}

produces output

a=7, b=5, c=3, d=1, e=-1

Should it not be acquiring ONE bit that is available on every core? Should the expected output not be

a=7, b=6, c=5, d=4, e=3

```c int main(void) { int a = multicore_doorbell_claim_unused(0x3, false); int b = multicore_doorbell_claim_unused(0x3, false); int c = multicore_doorbell_claim_unused(0x3, false); int d = multicore_doorbell_claim_unused(0x3, false); int e = multicore_doorbell_claim_unused(0x3, false); printf("a=%d, b=%d, c=%d, d=%d, e=%d\n", a, b, c, d, e); return 0; } ``` produces output ``` a=7, b=5, c=3, d=1, e=-1 ``` Should it not be acquiring ONE bit that is available on every core? Should the expected output not be ``` a=7, b=6, c=5, d=4, e=3 ``` ?

akohlsmith commented

2026-01-23 18:11:27 -06:00

(Migrated from github.com)

I pulled in the logic to a PC so I could get a better idea of what's going on.

const char *bin(uint8_t val)
{
    static char s[9];
    for (int i = 7; i >= 0; i--) {
        s[7 - i] = (val & (1 << i) ? '1' : '0');
    }

    s[8] = 0x00;
    return s;
}

static int claim_unused_and_check(uint core_mask)
{
    int ret;

    ret = multicore_doorbell_claim_unused(core_mask, false);
    printf("ret = %d, core0: 0b%s, core1: 0b%s\n", ret, bin(doorbell_claimed[0][0]), bin(doorbell_claimed[1][0]));

    return ret;
}


int main(void)
{
    int a = claim_unused_and_check(0x3);
    int b = claim_unused_and_check(0x3);
    int c = claim_unused_and_check(0x3);
    int d = claim_unused_and_check(0x3);
    int e = claim_unused_and_check(0x3);

    printf("a=%d, b=%d, c=%d, d=%d, e=%d\n", a, b, c, d, e);

    return 0;
}

produces the following output:

ret = 7, core0: 0b10000000, core1: 0b10000000
ret = 5, core0: 0b10100000, core1: 0b10100000
ret = 3, core0: 0b10101000, core1: 0b10101000
ret = 1, core0: 0b10101010, core1: 0b10101010
ret = -1, core0: 0b10101010, core1: 0b10101010
a=7, b=5, c=3, d=1, e=-1

The trouble appears to be in multicore_doorbell_claim_under_lock() where it's doing this:

uint claimed_cores_for_doorbell = (uint) (is_bit_claimed(doorbell_claimed[0], doorbell_num) |
     (is_bit_claimed(doorbell_claimed[1], doorbell_num + 1u) << 1));

which is building up a pair of claim bits, but the logic doesn't look right. It shouldn't be (doorbell_num + 1) << 1; the +1 is wrong. If I fix that line, the output of my test program now looks like this:

ret = 7, core0: 0b10000000, core1: 0b10000000
ret = 6, core0: 0b11000000, core1: 0b11000000
ret = 5, core0: 0b11100000, core1: 0b11100000
ret = 4, core0: 0b11110000, core1: 0b11110000
ret = 3, core0: 0b11111000, core1: 0b11111000
a=7, b=6, c=5, d=4, e=3

which matches my expectation.

I pulled in the logic to a PC so I could get a better idea of what's going on. ```c const char *bin(uint8_t val) { static char s[9]; for (int i = 7; i >= 0; i--) { s[7 - i] = (val & (1 << i) ? '1' : '0'); } s[8] = 0x00; return s; } static int claim_unused_and_check(uint core_mask) { int ret; ret = multicore_doorbell_claim_unused(core_mask, false); printf("ret = %d, core0: 0b%s, core1: 0b%s\n", ret, bin(doorbell_claimed[0][0]), bin(doorbell_claimed[1][0])); return ret; } int main(void) { int a = claim_unused_and_check(0x3); int b = claim_unused_and_check(0x3); int c = claim_unused_and_check(0x3); int d = claim_unused_and_check(0x3); int e = claim_unused_and_check(0x3); printf("a=%d, b=%d, c=%d, d=%d, e=%d\n", a, b, c, d, e); return 0; } ``` produces the following output: ``` ret = 7, core0: 0b10000000, core1: 0b10000000 ret = 5, core0: 0b10100000, core1: 0b10100000 ret = 3, core0: 0b10101000, core1: 0b10101000 ret = 1, core0: 0b10101010, core1: 0b10101010 ret = -1, core0: 0b10101010, core1: 0b10101010 a=7, b=5, c=3, d=1, e=-1 ``` The trouble appears to be in `multicore_doorbell_claim_under_lock()` where it's doing this: ```c uint claimed_cores_for_doorbell = (uint) (is_bit_claimed(doorbell_claimed[0], doorbell_num) | (is_bit_claimed(doorbell_claimed[1], doorbell_num + 1u) << 1)); ``` which is building up a pair of claim bits, but the logic doesn't look right. It shouldn't be `(doorbell_num + 1) << 1`; the `+1` is wrong. If I fix that line, the output of my test program now looks like this: ``` ret = 7, core0: 0b10000000, core1: 0b10000000 ret = 6, core0: 0b11000000, core1: 0b11000000 ret = 5, core0: 0b11100000, core1: 0b11100000 ret = 4, core0: 0b11110000, core1: 0b11110000 ret = 3, core0: 0b11111000, core1: 0b11111000 a=7, b=6, c=5, d=4, e=3 ``` which matches my expectation.