Page MenuHomeFreeBSD

buf_ring: Keep the full head and tail values
ClosedPublic

Authored by andrew on Jul 26 2024, 8:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 2, 5:20 PM
Unknown Object (File)
Wed, Oct 2, 7:57 AM
Unknown Object (File)
Sun, Sep 29, 10:30 PM
Unknown Object (File)
Fri, Sep 27, 11:01 AM
Unknown Object (File)
Mon, Sep 23, 10:19 PM
Unknown Object (File)
Sun, Sep 22, 10:59 PM
Unknown Object (File)
Sat, Sep 21, 9:10 AM
Unknown Object (File)
Sat, Sep 21, 9:09 AM
Subscribers

Details

Summary

If a thread reads the head but then sleeps for long enough that
another thread fills the ring and leaves the new head with the
expected value then the cmpset can pass when it should have failed.

To work around this keep the full head and tail value and use the
upper bits as a generation count.

Sponsored by: Arm Ltd

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58873
Build 55760: arc lint + arc unit

Event Timeline

sys/sys/buf_ring.h
85

If you define i in the loop header, the ifdef becomes a bit less ugly.

91

Is there some reason to load the mask in the critical section? It also duplicates the load in the debug code above.

Am I right that this revision does not fix the issue, but covers it with more bits from the unused space in the head index? It is still possible that head wraps around, just less likely now that full 2^32 iterations needs to occur under it?

In D46151#1052139, @kib wrote:

Am I right that this revision does not fix the issue, but covers it with more bits from the unused space in the head index? It is still possible that head wraps around, just less likely now that full 2^32 iterations needs to occur under it?

It's still possible, just very unlikely. Even if the value wrapped it would need to be the correct value for the cmpset to succeed.

In D46151#1052139, @kib wrote:

Am I right that this revision does not fix the issue, but covers it with more bits from the unused space in the head index? It is still possible that head wraps around, just less likely now that full 2^32 iterations needs to occur under it?

It's still possible, just very unlikely. Even if the value wrapped it would need to be the correct value for the cmpset to succeed.

I suggest adding a comment, to save the future reader from realizing this on his own,

This revision is now accepted and ready to land.Aug 16 2024, 8:10 AM
This revision was automatically updated to reflect the committed changes.