Skip to content

Commit

Permalink
Revert "Kernel: Make DoubleBuffer use a KBuffer instead of kmalloc()ing"
Browse files Browse the repository at this point in the history
This reverts commit 1cca514.

This appears to be causing intermittent triple-faults and I don't know
why yet, so I'll just revert it to keep the tree in decent shape.
  • Loading branch information
awesomekling committed Oct 18, 2019
1 parent 1cca514 commit ec65b8d
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 53 deletions.
45 changes: 15 additions & 30 deletions Kernel/DoubleBuffer.cpp
Original file line number Diff line number Diff line change
@@ -1,44 +1,29 @@
#include <Kernel/DoubleBuffer.h>

inline void DoubleBuffer::compute_lockfree_metadata()
inline void DoubleBuffer::compute_emptiness()
{
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))
, 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;
m_empty = m_read_buffer_index >= m_read_buffer->size() && m_write_buffer->is_empty();
}

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);
m_write_buffer->size = 0;
if (m_write_buffer->capacity() < 32)
m_write_buffer->clear_with_capacity();
else
m_write_buffer->clear();
m_read_buffer_index = 0;
compute_lockfree_metadata();
compute_emptiness();
}

ssize_t DoubleBuffer::write(const u8* data, ssize_t size)
{
if (!size)
return 0;
LOCKER(m_lock);
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);
m_write_buffer->append(data, size);
compute_emptiness();
return size;
}

Expand All @@ -47,13 +32,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->size != 0)
if (m_read_buffer_index >= m_read_buffer->size() && !m_write_buffer->is_empty())
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 - (ssize_t)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() - m_read_buffer_index, size);
memcpy(data, m_read_buffer->data() + m_read_buffer_index, nread);
m_read_buffer_index += nread;
compute_lockfree_metadata();
compute_emptiness();
return nread;
}
34 changes: 15 additions & 19 deletions Kernel/DoubleBuffer.h
Original file line number Diff line number Diff line change
@@ -1,38 +1,34 @@
#pragma once

#include <AK/Types.h>
#include <Kernel/KBuffer.h>
#include <AK/Vector.h>
#include <Kernel/Lock.h>

class DoubleBuffer {
public:
explicit DoubleBuffer(size_t capacity = 65536);
DoubleBuffer()
: m_write_buffer(&m_buffer1)
, m_read_buffer(&m_buffer2)
{
}

ssize_t write(const u8*, ssize_t);
ssize_t read(u8*, ssize_t);

bool is_empty() const { return m_empty; }

size_t space_for_writing() const { return m_space_for_writing; }
// 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(); }

private:
void flip();
void compute_lockfree_metadata();
void compute_emptiness();

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 };
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 };
mutable Lock m_lock { "DoubleBuffer" };
Lock m_lock { "DoubleBuffer" };
};
2 changes: 1 addition & 1 deletion Kernel/FileSystem/FIFO.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ bool FIFO::can_read(FileDescription&) const

bool FIFO::can_write(FileDescription&) const
{
return m_buffer.space_for_writing() || !m_readers;
return m_buffer.bytes_in_write_buffer() < 4096 || !m_readers;
}

ssize_t FIFO::read(FileDescription&, u8* buffer, ssize_t size)
Expand Down
4 changes: 2 additions & 2 deletions Kernel/Net/LocalSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ bool LocalSocket::can_write(FileDescription& description) const
{
auto role = this->role(description);
if (role == Role::Accepted)
return !has_attached_peer(description) || m_for_client.space_for_writing();
return !has_attached_peer(description) || m_for_client.bytes_in_write_buffer() < 16384;
if (role == Role::Connected)
return !has_attached_peer(description) || m_for_server.space_for_writing();
return !has_attached_peer(description) || m_for_server.bytes_in_write_buffer() < 16384;
ASSERT_NOT_REACHED();
}

Expand Down
2 changes: 1 addition & 1 deletion Kernel/TTY/MasterPTY.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ bool MasterPTY::can_write_from_slave() const
{
if (m_closed)
return true;
return m_buffer.space_for_writing();
return m_buffer.bytes_in_write_buffer() < 4096;
}

void MasterPTY::close()
Expand Down

0 comments on commit ec65b8d

Please sign in to comment.