Kernel: Make DoubleBuffer use a KBuffer instead of kmalloc()ing

Background: DoubleBuffer is a handy buffer class in the kernel that
allows you to keep writing to it from the "outside" while the "inside"
reads from it. It's used for things like LocalSocket and TTY's.
Internally, it has a read buffer and a write buffer, but the two will
swap places when the read buffer is exhausted (by reading from it.)

Before this patch, it was internally implemented as two Vector<u8>
that we would swap between when the reader side had exhausted the data
in the read buffer. Now instead we preallocate a large KBuffer (64KB*2)
on DoubleBuffer construction and use that throughout its lifetime.

This removes all the kmalloc heap traffic caused by DoubleBuffers :^)

Kernel/DoubleBuffer.cpp

@@ -26,21 +26,33 @@
 
 #include <Kernel/DoubleBuffer.h>
 
-inline void DoubleBuffer::compute_emptiness()
+inline void DoubleBuffer::compute_lockfree_metadata()
 {
-    m_empty = m_read_buffer_index >= m_read_buffer->size() && m_write_buffer->is_empty();
+    InterruptDisabler disabler;
+    m_empty = m_read_buffer_index >= m_read_buffer->size && m_write_buffer->size == 0;
+    m_space_for_writing = m_capacity - m_write_buffer->size;
+}
+
+DoubleBuffer::DoubleBuffer(size_t capacity)
+    : m_write_buffer(&m_buffer1)
+    , m_read_buffer(&m_buffer2)
+    , m_storage(KBuffer::create_with_size(capacity * 2, Region::Access::Read | Region::Access::Write, "DoubleBuffer"))
+    , m_capacity(capacity)
+{
+    m_buffer1.data = m_storage.data();
+    m_buffer1.size = 0;
+    m_buffer2.data = m_storage.data() + capacity;
+    m_buffer2.size = 0;
+    m_space_for_writing = capacity;
 }
 
 void DoubleBuffer::flip()
 {
-    ASSERT(m_read_buffer_index == m_read_buffer->size());
+    ASSERT(m_read_buffer_index == m_read_buffer->size);
     swap(m_read_buffer, m_write_buffer);
-    if (m_write_buffer->capacity() < 32)
-        m_write_buffer->clear_with_capacity();
-    else
-        m_write_buffer->clear();
+    m_write_buffer->size = 0;
     m_read_buffer_index = 0;
-    compute_emptiness();
+    compute_lockfree_metadata();
 }
 
 ssize_t DoubleBuffer::write(const u8* data, ssize_t size)
@@ -48,8 +60,11 @@ ssize_t DoubleBuffer::write(const u8* data, ssize_t size)
     if (!size)
         return 0;
     LOCKER(m_lock);
-    m_write_buffer->append(data, size);
-    compute_emptiness();
+    ASSERT(size <= (ssize_t)space_for_writing());
+    u8* write_ptr = m_write_buffer->data + m_write_buffer->size;
+    m_write_buffer->size += size;
+    compute_lockfree_metadata();
+    memcpy(write_ptr, data, size);
     return size;
 }
 
@@ -58,13 +73,13 @@ ssize_t DoubleBuffer::read(u8* data, ssize_t size)
     if (!size)
         return 0;
     LOCKER(m_lock);
-    if (m_read_buffer_index >= m_read_buffer->size() && !m_write_buffer->is_empty())
+    if (m_read_buffer_index >= m_read_buffer->size && m_write_buffer->size != 0)
         flip();
-    if (m_read_buffer_index >= m_read_buffer->size())
+    if (m_read_buffer_index >= m_read_buffer->size)
         return 0;
-    ssize_t nread = min((ssize_t)m_read_buffer->size() - m_read_buffer_index, size);
-    memcpy(data, m_read_buffer->data() + m_read_buffer_index, nread);
+    ssize_t nread = min((ssize_t)m_read_buffer->size - (ssize_t)m_read_buffer_index, size);
+    memcpy(data, m_read_buffer->data + m_read_buffer_index, nread);
     m_read_buffer_index += nread;
-    compute_emptiness();
+    compute_lockfree_metadata();
     return nread;
 }

Kernel/DoubleBuffer.h

@@ -27,34 +27,38 @@
 #pragma once
 
 #include <AK/Types.h>
-#include <AK/Vector.h>
+#include <Kernel/KBuffer.h>
 #include <Kernel/Lock.h>
 
 class DoubleBuffer {
 public:
-    DoubleBuffer()
-        : m_write_buffer(&m_buffer1)
-        , m_read_buffer(&m_buffer2)
-    {
-    }
+    explicit DoubleBuffer(size_t capacity = 65536);
 
     ssize_t write(const u8*, ssize_t);
     ssize_t read(u8*, ssize_t);
 
     bool is_empty() const { return m_empty; }
 
-    // FIXME: Isn't this racy? What if we get interrupted between getting the buffer pointer and dereferencing it?
-    ssize_t bytes_in_write_buffer() const { return (ssize_t)m_write_buffer->size(); }
+    size_t space_for_writing() const { return m_space_for_writing; }
 
 private:
     void flip();
-    void compute_emptiness();
+    void compute_lockfree_metadata();
 
-    Vector<u8>* m_write_buffer { nullptr };
-    Vector<u8>* m_read_buffer { nullptr };
-    Vector<u8> m_buffer1;
-    Vector<u8> m_buffer2;
-    ssize_t m_read_buffer_index { 0 };
-    bool m_empty { true };
-    Lock m_lock { "DoubleBuffer" };
+    struct InnerBuffer {
+        u8* data { nullptr };
+        size_t size;
+    };
+
+    InnerBuffer* m_write_buffer { nullptr };
+    InnerBuffer* m_read_buffer { nullptr };
+    InnerBuffer m_buffer1;
+    InnerBuffer m_buffer2;
+
+    KBuffer m_storage;
+    size_t m_capacity { 0 };
+    size_t m_read_buffer_index { 0 };
+    size_t m_space_for_writing { 0 };
+    bool m_empty { true };
+    mutable Lock m_lock { "DoubleBuffer" };
 };

Kernel/FileSystem/FIFO.cpp

@@ -110,7 +110,7 @@ bool FIFO::can_read(const FileDescription&) const
 
 bool FIFO::can_write(const FileDescription&) const
 {
-    return m_buffer.bytes_in_write_buffer() < 4096 || !m_readers;
+    return m_buffer.space_for_writing() || !m_readers;
 }
 
 ssize_t FIFO::read(FileDescription&, u8* buffer, ssize_t size)

Kernel/Net/IPv4Socket.cpp

@@ -363,9 +363,7 @@ bool IPv4Socket::did_receive(const IPv4Address& source_address, u16 source_port,
     auto packet_size = packet.size();
 
     if (buffer_mode() == BufferMode::Bytes) {
-        constexpr size_t max_buffer_amount = 128 * KB;
-        ASSERT((size_t)m_receive_buffer.bytes_in_write_buffer() < max_buffer_amount);
-        size_t space_in_receive_buffer = max_buffer_amount - (size_t)m_receive_buffer.bytes_in_write_buffer();
+        size_t space_in_receive_buffer = m_receive_buffer.space_for_writing();
         if (packet_size > space_in_receive_buffer) {
             kprintf("IPv4Socket(%p): did_receive refusing packet since buffer is full.\n", this);
             ASSERT(m_can_read);

Kernel/Net/LocalSocket.cpp

@@ -250,9 +250,9 @@ bool LocalSocket::can_write(const FileDescription& description) const
 {
     auto role = this->role(description);
     if (role == Role::Accepted)
-        return !has_attached_peer(description) || m_for_client.bytes_in_write_buffer() < 16384;
+        return !has_attached_peer(description) || m_for_client.space_for_writing();
     if (role == Role::Connected)
-        return !has_attached_peer(description) || m_for_server.bytes_in_write_buffer() < 16384;
+        return !has_attached_peer(description) || m_for_server.space_for_writing();
     return false;
 }
 

Kernel/TTY/MasterPTY.cpp

@@ -107,7 +107,7 @@ bool MasterPTY::can_write_from_slave() const
 {
     if (m_closed)
         return true;
-    return m_buffer.bytes_in_write_buffer() < 4096;
+    return m_buffer.space_for_writing();
 }
 
 void MasterPTY::close()