Skip to content

Commit 52a2fcc

Browse files
spikehmeta-codesync[bot]
authored andcommitted
IoUringDynamicProvidedBufferRing: fix empty-ring area handoff
Summary: When the dynamic provided buffer ring reaches its area limit while buffers are still held, the kernel ring can drain completely. If an older area subsequently becomes reusable, refill selects it as `bufferRefillArea_`, but `bufferActiveArea_` remains on the previously exhausted area because no completion remains to perform the normal end-of-area handoff. The next completion then wraps memory from the stale active area instead of the area posted to the kernel. Returns are also charged to the wrong area, which can allow an area with live IOBufs to be reclaimed. When refilling an empty ring, make the selected refill area active before publishing its descriptors. Reviewed By: jlhe97 Differential Revision: D116800599 fbshipit-source-id: fcb815fa1ae1afa5323e1ae4bdd7ceb8f5cf3908
1 parent 00576b7 commit 52a2fcc

2 files changed

Lines changed: 39 additions & 0 deletions

File tree

third-party/folly/src/folly/io/async/IoUringDynamicProvidedBufferRing.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ void IoUringDynamicProvidedBufferRing::ringRefill() noexcept {
272272
return;
273273
}
274274

275+
if (ringFillLevel() == 0) {
276+
bufferActiveArea_ = bufferRefillArea_;
277+
}
278+
275279
uint16_t pendingOutstanding = 0;
276280
auto freeEntries = ringFreeEntries();
277281
while (freeEntries--) {

third-party/folly/src/folly/io/async/test/IoUringDynamicProvidedBufferRingTest.cpp

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ class IoUringDynamicProvidedBufferRingTestHelper {
5757
uint32_t ringAvailable() { return ring.ringTail_ - ring.ringHead_; }
5858
uint64_t outstandingSum() { return ring.areasOutstandingSum(); }
5959
uint16_t headBid() { return ring.ringBuf(ring.ringHead_)->bid; }
60+
const unsigned char* headAddr() {
61+
return reinterpret_cast<const unsigned char*>(
62+
ring.ringBuf(ring.ringHead_)->addr);
63+
}
6064

6165
void setRingRefillThreshold(uint16_t threshold) {
6266
ring.ringRefillThreshold_ = threshold;
@@ -725,4 +729,35 @@ TEST_F(
725729
<< "threshold consumption should trigger a batched refill";
726730
}
727731

732+
TEST_F(
733+
IoUringDynamicProvidedBufferRingTest, RefillAfterAreaCapUsesRecycledArea) {
734+
io_uring ring{};
735+
io_uring_queue_init(512, &ring, 0);
736+
IoUringDynamicProvidedBufferRing::Options options = {
737+
.gid = 1,
738+
.bufferCount = 2,
739+
.bufferSize = 64,
740+
};
741+
auto bufRing = IoUringDynamicProvidedBufferRing::create(&ring, options);
742+
IoUringDynamicProvidedBufferRingTestHelper helper(*bufRing);
743+
744+
std::vector<std::unique_ptr<folly::IOBuf>> held;
745+
for (int i = 0; i < 1000 && helper.ringAvailable() > 0; i++) {
746+
held.push_back(consumeOne(*bufRing, helper, 64));
747+
}
748+
ASSERT_EQ(helper.areaCount(), 64u);
749+
ASSERT_EQ(helper.ringAvailable(), 0u);
750+
751+
held.erase(held.begin(), held.begin() + options.bufferCount);
752+
753+
bufRing->enobuf();
754+
ASSERT_EQ(helper.ringAvailable(), options.bufferCount);
755+
756+
for (uint32_t i = 0; i < options.bufferCount; i++) {
757+
const auto* posted = helper.headAddr();
758+
auto buf = consumeOne(*bufRing, helper, 64);
759+
EXPECT_EQ(buf->data(), posted);
760+
}
761+
}
762+
728763
#endif

0 commit comments

Comments
 (0)