Open timvisee opened 4 years ago
Ping @jamesmunns (due to draft PR)
This uses fetch_or and fetch_add, do we need to implement this for thumbv6?
Apparently, yes: https://github.com/jamesmunns/bbqueue/runs/1390103704
Edit: this is now implemented
Do we still need to zero the buffer when taking just the producer or consumer, like try_split? (currently commented-out)
Yes, I would suggest doing either on the first one that is taken, or doing it for the consumer alone is probably enough. Otherwise it is undefined behavior.
Do we still need to reinitialize the buffer when releasing just the producer or consumer, like try_release?
This is optional. It's generally not safe to do unless BOTH halves have been released. When in doubt, skip it.
Do we still need to zero the buffer when taking just the producer or consumer, like try_split? (currently commented-out)
Yes, I would suggest doing either on the first one that is taken, or doing it for the consumer alone is probably enough. Otherwise it is undefined behavior.
Do we still need to reinitialize the buffer when releasing just the producer or consumer, like try_release?
This is optional. It's generally not safe to do unless BOTH halves have been released. When in doubt, skip it.
This has been implemented in de43a301adc7eaae872f782a5c0ca686b07b8da0
Zeroes on try_take_consumer
. Reinitializes when both the producer and consumer are released.
Hey @timvisee, I'll plan on reviewing this PR this weekend, but I realized that I was wrong - you'll need to zero-init the buffer when the PRODUCER (not consumer) is taken - as the producer will be the one to "see" the contents of the data first (e.g. if a producer is taken, then a grant is requested, the grant will be able to "see" uninitialized data if no consumer has been taken). Sorry for the wrong guidance before.
@jamesmunns It now zeroes when the producer is taken. I amended this change to the last commit (03b97746e5af06c82f6a10841c54e2601a14ab5d).
Will conflict with #103
This implements:
BBBuffer::try_take_producer
: split-off just the producerBBBuffer::try_take_consumer
: split-off just the consumerBBBuffer::try_release_producer
: release (give back) just the producerBBBuffer::try_release_consumer
: release (give back) just the consumerI choose to use an
AtomicU8
for thealready_split
state with a bit for the producer and consumer parts which are defined inBIT_PRODUCER
andBIT_CONSUMER
respectively. I thought this would be the right approach as updating the state for both the producer and consumer can still be done in a single atomic operation. That wouldn't be the case when using two separateAtomicBool
's.Things to resolve before merge:
try_split
? (currently commented-out)try_release
?fetch_or
andfetch_add
, do we need to implement this forthumbv6
?The above things I'm currently uncertain about. My knowledge is lacking on this, thus I'd like to ask to make sure the implementation is sound. The PR branch is open, so feel free to make an edit.
Fixes https://github.com/jamesmunns/bbqueue/issues/40
Related https://github.com/jamesmunns/bbqueue/issues/67