VFS: Replace global response handlers with local handlers

Replace the I/O response handler that is passed to the VFS at
construction with an object that is dynamically attached to handles.
This object shall also accept read-ready notifications, and plugins are
encouraged to keep handles awaiting ready-ready notifications separate
from handles that await I/O progress.

Replace the use of handle lists in plugins with handle queues, this
makes the code easier to understand and the ordering of notifications to
the application more explicit.

These changes replace the use of the Post_signal_hook from all VFS
plugins, applications must assume that read-ready and I/O notifications
occur during I/O signal dispatch and use an Io_progress_handler at its
entrypoints to defer response until after signal dispatching.

Fix #3257
This commit is contained in:
Emery Hemingway
2019-03-25 15:41:43 +01:00
committed by Christian Helmuth
parent e2ff776b35
commit a635873568
27 changed files with 1397 additions and 1341 deletions

View File

@ -21,7 +21,6 @@
#include <base/id_space.h>
#include <file_system_session/connection.h>
namespace Vfs { class Fs_file_system; }
@ -67,15 +66,18 @@ class Vfs::Fs_file_system : public File_system
::File_system::Packet_descriptor queued_sync_packet { };
};
struct Fs_vfs_handle;
typedef Genode::Fifo<Fs_vfs_handle> Fs_vfs_handle_queue;
struct Fs_vfs_handle : Vfs_handle,
private ::File_system::Node,
private Handle_space::Element,
private List<Fs_vfs_handle>::Element,
private Fs_vfs_handle_queue::Element,
private Handle_state
{
friend Genode::Id_space<::File_system::Node>;
friend Genode::List<Fs_vfs_handle>;
using Genode::List<Fs_vfs_handle>::Element::next;
friend Fs_vfs_handle_queue;
using Fs_vfs_handle_queue::Element::enqueued;
using Handle_state::queued_read_state;
using Handle_state::queued_read_packet;
@ -84,7 +86,6 @@ class Vfs::Fs_file_system : public File_system
using Handle_state::read_ready_state;
::File_system::Connection &_fs;
Io_response_handler &_io_handler;
bool _queue_read(file_size count, file_size const seek_offset)
{
@ -94,7 +95,8 @@ class Vfs::Fs_file_system : public File_system
::File_system::Session::Tx::Source &source = *_fs.tx();
/* if not ready to submit suggest retry */
if (!source.ready_to_submit()) return false;
if (!source.ready_to_submit())
return false;
file_size const max_packet_size = source.bulk_buffer_size() / 2;
file_size const clipped_count = min(max_packet_size, count);
@ -149,12 +151,11 @@ class Vfs::Fs_file_system : public File_system
Fs_vfs_handle(File_system &fs, Allocator &alloc,
int status_flags, Handle_space &space,
::File_system::Node_handle node_handle,
::File_system::Connection &fs_connection,
Io_response_handler &io_handler)
::File_system::Connection &fs_connection)
:
Vfs_handle(fs, fs, alloc, status_flags),
Handle_space::Element(*this, space, node_handle),
_fs(fs_connection), _io_handler(io_handler)
_fs(fs_connection)
{ }
::File_system::File_handle file_handle() const
@ -334,11 +335,10 @@ class Vfs::Fs_file_system : public File_system
::File_system::Session &fs_session,
::File_system::Node_handle fs_handle,
Handle_space &space,
::File_system::Connection &fs_connection,
Io_response_handler &io_handler)
::File_system::Connection &fs_connection)
:
Fs_vfs_handle(fs, *(Allocator*)nullptr, 0, space, fs_handle,
fs_connection, io_handler),
fs_connection),
_fs_session(fs_session)
{ }
@ -350,12 +350,9 @@ class Vfs::Fs_file_system : public File_system
struct Fs_vfs_watch_handle final : Vfs_watch_handle,
private ::File_system::Node,
private Handle_space::Element,
private List<Fs_vfs_watch_handle>::Element
private Handle_space::Element
{
friend Genode::Id_space<::File_system::Node>;
friend Genode::List<Fs_vfs_watch_handle>;
using Genode::List<Fs_vfs_watch_handle>::Element::next;
::File_system::Watch_handle const fs_handle;
@ -370,77 +367,7 @@ class Vfs::Fs_file_system : public File_system
{ }
};
struct Post_signal_hook : Genode::Entrypoint::Post_signal_hook
{
Genode::Entrypoint &_ep;
Io_response_handler &_io_handler;
Watch_response_handler &_watch_handler;
List<Fs_vfs_handle> _io_handle_list { };
Lock _list_lock { };
bool _notify_all { false };
Post_signal_hook(Vfs::Env &env)
:
_ep(env.env().ep()),
_io_handler(env.io_handler()),
_watch_handler(env.watch_handler())
{ }
void arm_io_event(Fs_vfs_handle *context)
{
if (!context) {
Lock::Guard list_guard(_list_lock);
_notify_all = true;
} else {
Lock::Guard list_guard(_list_lock);
for (Fs_vfs_handle *list_context = _io_handle_list.first();
list_context;
list_context = list_context->next())
{
if (list_context == context) {
/* already in list */
return;
}
}
_io_handle_list.insert(context);
}
_ep.schedule_post_signal_hook(this);
}
void function() override
{
Fs_vfs_handle *handle = nullptr;
do {
bool notify_all = false;
{
Lock::Guard list_guard(_list_lock);
handle = _io_handle_list.first();
_io_handle_list.remove(handle);
if (!handle && _notify_all) {
notify_all = true;
_notify_all = false;
}
}
if (handle) {
_io_handler.handle_io_response(handle->context());
} else if (notify_all) {
_io_handler.handle_io_response(nullptr);
}
/* done if no contexts and all notified */
} while (handle);
}
};
Post_signal_hook _post_signal_hook { _env };
Fs_vfs_handle_queue _congested_handles { };
file_size _read(Fs_vfs_handle &handle, void *buf,
file_size const count, file_size const seek_offset)
@ -465,9 +392,8 @@ class Vfs::Fs_file_system : public File_system
/* pass packet to server side */
source.submit_packet(packet_in);
while (handle.queued_read_state != Handle_state::Queued_state::ACK) {
while (handle.queued_read_state != Handle_state::Queued_state::ACK)
_env.env().ep().wait_and_dispatch_one_io_signal();
}
/* obtain result packet descriptor with updated status info */
Packet_descriptor const packet_out = handle.queued_read_packet;
@ -498,14 +424,25 @@ class Vfs::Fs_file_system : public File_system
file_size _write(Fs_vfs_handle &handle,
const char *buf, file_size count, file_size seek_offset)
{
/*
* TODO
* a sustained write loop will congest the packet buffer,
* perhaps acks should be processed before submission?
*
* _handle_ack();
*/
::File_system::Session::Tx::Source &source = *_fs.tx();
using ::File_system::Packet_descriptor;
file_size const max_packet_size = source.bulk_buffer_size() / 2;
count = min(max_packet_size, count);
if (!source.ready_to_submit())
if (!source.ready_to_submit()) {
if (!handle.enqueued())
_congested_handles.enqueue(handle);
throw Insufficient_buffer();
}
try {
Packet_descriptor packet_in(source.alloc_packet(count),
@ -519,6 +456,8 @@ class Vfs::Fs_file_system : public File_system
/* pass packet to server side */
source.submit_packet(packet_in);
} catch (::File_system::Session::Tx::Source::Packet_alloc_failed) {
if (!handle.enqueued())
_congested_handles.enqueue(handle);
throw Insufficient_buffer();
} catch (...) {
Genode::error("unhandled exception");
@ -529,8 +468,8 @@ class Vfs::Fs_file_system : public File_system
void _ready_to_submit()
{
/* notify anyone who might have failed on write() ready_to_submit */
_post_signal_hook.arm_io_event(nullptr);
_congested_handles.dequeue_all([] (Fs_vfs_handle &handle) {
handle.io_progress_response(); });
}
void _handle_ack()
@ -552,13 +491,13 @@ class Vfs::Fs_file_system : public File_system
switch (packet.operation()) {
case Packet_descriptor::READ_READY:
handle.read_ready_state = Handle_state::Read_ready_state::READY;
_post_signal_hook.arm_io_event(&handle);
handle.read_ready_response();
break;
case Packet_descriptor::READ:
handle.queued_read_packet = packet;
handle.queued_read_state = Handle_state::Queued_state::ACK;
_post_signal_hook.arm_io_event(&handle);
handle.io_progress_response();
break;
case Packet_descriptor::WRITE:
@ -566,13 +505,13 @@ class Vfs::Fs_file_system : public File_system
* Notify anyone who might have failed on
* 'alloc_packet()'
*/
_post_signal_hook.arm_io_event(nullptr);
handle.io_progress_response();
break;
case Packet_descriptor::SYNC:
handle.queued_sync_packet = packet;
handle.queued_sync_state = Handle_state::Queued_state::ACK;
_post_signal_hook.arm_io_event(&handle);
handle.io_progress_response();
break;
case Packet_descriptor::CONTENT_CHANGED:
@ -583,20 +522,14 @@ class Vfs::Fs_file_system : public File_system
try {
if (packet.operation() == Packet_descriptor::CONTENT_CHANGED) {
/*
* Trigger the watch response during signal dispatch.
* This is incompatible with the Libc I/O handling
* but the Libc does not open watch handles and shall
* not use them before Post_signal_hook is removed.
*/
_watch_handle_space.apply<Fs_vfs_watch_handle>(id, [&] (Fs_vfs_watch_handle &handle) {
_env.watch_handler().handle_watch_response(handle.context()); });
handle.watch_response(); });
} else {
_handle_space.apply<Fs_vfs_handle>(id, handle_read);
}
}
catch (Handle_space::Unknown_id) {
Genode::warning("ack for unknown VFS handle"); }
Genode::warning("ack for unknown File_system handle ", id); }
if (packet.operation() == Packet_descriptor::WRITE) {
Lock::Guard guard(_lock);
@ -605,8 +538,16 @@ class Vfs::Fs_file_system : public File_system
}
}
void _handle_ack_signal()
{
_handle_ack();
/* packet buffer space available */
_ready_to_submit();
}
Genode::Io_signal_handler<Fs_file_system> _ack_handler {
_env.env().ep(), *this, &Fs_file_system::_handle_ack };
_env.env().ep(), *this, &Fs_file_system::_handle_ack_signal };
Genode::Io_signal_handler<Fs_file_system> _ready_handler {
_env.env().ep(), *this, &Fs_file_system::_ready_to_submit };
@ -652,12 +593,17 @@ class Vfs::Fs_file_system : public File_system
try {
::File_system::Node_handle node = _fs.node(path);
Fs_handle_guard node_guard(*this, _fs, node, _handle_space,
_fs, _env.io_handler());
Fs_handle_guard node_guard(*this, _fs, node, _handle_space, _fs);
status = _fs.status(node);
}
catch (Genode::Out_of_ram) { return STAT_ERR_NO_PERM; }
catch (Genode::Out_of_caps) { return STAT_ERR_NO_PERM; }
catch (Genode::Out_of_ram) {
Genode::error("out-of-ram during stat");
return STAT_ERR_NO_PERM;
}
catch (Genode::Out_of_caps) {
Genode::error("out-of-caps during stat");
return STAT_ERR_NO_PERM;
}
catch (...) { return STAT_ERR_NO_ENTRY; }
out = Stat();
@ -688,8 +634,7 @@ class Vfs::Fs_file_system : public File_system
try {
::File_system::Dir_handle dir = _fs.dir(dir_path.base(), false);
Fs_handle_guard dir_guard(*this, _fs, dir, _handle_space, _fs,
_env.io_handler());
Fs_handle_guard dir_guard(*this, _fs, dir, _handle_space, _fs);
_fs.unlink(dir, file_name.base() + 1);
}
@ -725,12 +670,12 @@ class Vfs::Fs_file_system : public File_system
_fs.dir(from_dir_path.base(), false);
Fs_handle_guard from_dir_guard(*this, _fs, from_dir,
_handle_space, _fs, _env.io_handler());
_handle_space, _fs);
::File_system::Dir_handle to_dir = _fs.dir(to_dir_path.base(),
false);
Fs_handle_guard to_dir_guard(*this, _fs, to_dir, _handle_space,
_fs, _env.io_handler());
Fs_handle_guard to_dir_guard(
*this, _fs, to_dir, _handle_space, _fs);
_fs.move(from_dir, from_file_name.base() + 1,
to_dir, to_file_name.base() + 1);
@ -749,9 +694,10 @@ class Vfs::Fs_file_system : public File_system
try {
::File_system::Node_handle node = _fs.node(path);
Fs_handle_guard node_guard(*this, _fs, node,
_handle_space, _fs,
_env.io_handler());
_handle_space, _fs);
::File_system::Status status = _fs.status(node);
return status.size / sizeof(::File_system::Directory_entry);
}
catch (...) { }
@ -762,8 +708,7 @@ class Vfs::Fs_file_system : public File_system
{
try {
::File_system::Node_handle node = _fs.node(path);
Fs_handle_guard node_guard(*this, _fs, node, _handle_space,
_fs, _env.io_handler());
Fs_handle_guard node_guard(*this, _fs, node, _handle_space, _fs);
::File_system::Status status = _fs.status(node);
@ -809,16 +754,14 @@ class Vfs::Fs_file_system : public File_system
try {
::File_system::Dir_handle dir = _fs.dir(dir_path.base(), false);
Fs_handle_guard dir_guard(*this, _fs, dir, _handle_space, _fs,
_env.io_handler());
Fs_handle_guard dir_guard(*this, _fs, dir, _handle_space, _fs);
::File_system::File_handle file = _fs.file(dir,
file_name.base() + 1,
mode, create);
*out_handle = new (alloc)
Fs_vfs_file_handle(*this, alloc, vfs_mode, _handle_space,
file, _fs, _env.io_handler());
Fs_vfs_file_handle(*this, alloc, vfs_mode, _handle_space, file, _fs);
}
catch (::File_system::Lookup_failed) { return OPEN_ERR_UNACCESSIBLE; }
catch (::File_system::Permission_denied) { return OPEN_ERR_NO_PERM; }
@ -846,7 +789,7 @@ class Vfs::Fs_file_system : public File_system
*out_handle = new (alloc)
Fs_vfs_dir_handle(*this, alloc, ::File_system::READ_ONLY,
_handle_space, dir, _fs, _env.io_handler());
_handle_space, dir, _fs);
}
catch (::File_system::Lookup_failed) { return OPENDIR_ERR_LOOKUP_FAILED; }
catch (::File_system::Name_too_long) { return OPENDIR_ERR_NAME_TOO_LONG; }
@ -878,7 +821,7 @@ class Vfs::Fs_file_system : public File_system
false);
Fs_handle_guard from_dir_guard(*this, _fs, dir_handle,
_handle_space, _fs, _env.io_handler());
_handle_space, _fs);
::File_system::Symlink_handle symlink_handle =
_fs.symlink(dir_handle, symlink_name.base() + 1, create);
@ -886,8 +829,7 @@ class Vfs::Fs_file_system : public File_system
*out_handle = new (alloc)
Fs_vfs_symlink_handle(*this, alloc,
::File_system::READ_ONLY,
_handle_space, symlink_handle, _fs,
_env.io_handler());
_handle_space, symlink_handle, _fs);
return OPENLINK_OK;
}
@ -907,6 +849,8 @@ class Vfs::Fs_file_system : public File_system
Lock::Guard guard(_lock);
Fs_vfs_handle *fs_handle = static_cast<Fs_vfs_handle *>(vfs_handle);
if (fs_handle->enqueued())
_congested_handles.remove(*fs_handle);
_fs.close(fs_handle->file_handle());
destroy(fs_handle->alloc(), fs_handle);
@ -968,7 +912,6 @@ class Vfs::Fs_file_system : public File_system
Fs_vfs_handle &handle = static_cast<Fs_vfs_handle &>(*vfs_handle);
out_count = _write(handle, buf, buf_size, handle.seek());
return WRITE_OK;
}
@ -978,7 +921,10 @@ class Vfs::Fs_file_system : public File_system
Fs_vfs_handle *handle = static_cast<Fs_vfs_handle *>(vfs_handle);
return handle->queue_read(count);
bool result = handle->queue_read(count);
if (!result && !handle->enqueued())
_congested_handles.enqueue(*handle);
return result;
}
Read_result complete_read(Vfs_handle *vfs_handle, char *dst, file_size count,
@ -990,7 +936,10 @@ class Vfs::Fs_file_system : public File_system
Fs_vfs_handle *handle = static_cast<Fs_vfs_handle *>(vfs_handle);
return handle->complete_read(dst, count, out_count);
Read_result result = handle->complete_read(dst, count, out_count);
if (result == READ_QUEUED && !handle->enqueued())
_congested_handles.enqueue(*handle);
return result;
}
bool read_ready(Vfs_handle *vfs_handle) override
@ -1024,7 +973,7 @@ class Vfs::Fs_file_system : public File_system
/*
* When the packet is acknowledged the application is notified via
* Io_response_handler::handle_io_response().
* Response_handler::handle_response().
*/
return true;
}