vfs: no 'handle_io_response()' in regular VFS functions

Calling 'handle_io_response()' in a regular VFS function (in contrast to a
post-signal hook) can cause problems if the caller of the VFS function holds
a lock which prevents the io response handler from returning.

With this commit, the user of the VFS becomes responsible for unblocking
threads which might be blocking after a failed 'queue_read()', 'queue_sync()'
or 'write()' call.

Fixes #2896

repos/libports/src/lib/libc/vfs_plugin.cc

@@ -422,6 +422,10 @@ ssize_t Libc::Vfs_plugin::write(Libc::File_descriptor *fd, const void *buf,
 
 		try {
 			out_result = VFS_THREAD_SAFE(handle->fs().write(handle, (char const *)buf, count, out_count));
+
+			/* wake up threads blocking for 'queue_*()' or 'write()' */
+			Libc::resume_all();
+
 		} catch (Vfs::File_io_service::Insufficient_buffer) { }
 
 	} else {
@@ -462,6 +466,9 @@ ssize_t Libc::Vfs_plugin::write(Libc::File_descriptor *fd, const void *buf,
 		} while (check.retry);
 	}
 
+	/* wake up threads blocking for 'queue_*()' or 'write()' */
+	Libc::resume_all();
+
 	switch (out_result) {
 	case Result::WRITE_ERR_AGAIN:       return Errno(EAGAIN);
 	case Result::WRITE_ERR_WOULD_BLOCK: return Errno(EWOULDBLOCK);
@@ -552,6 +559,9 @@ ssize_t Libc::Vfs_plugin::read(Libc::File_descriptor *fd, void *buf,
 		} while (check.retry);
 	}
 
+	/* wake up threads blocking for 'queue_*()' or 'write()' */
+	Libc::resume_all();
+
 	switch (out_result) {
 	case Result::READ_ERR_AGAIN:       return Errno(EAGAIN);
 	case Result::READ_ERR_WOULD_BLOCK: return Errno(EWOULDBLOCK);
@@ -645,6 +655,9 @@ ssize_t Libc::Vfs_plugin::getdirentries(Libc::File_descriptor *fd, char *buf,
 		} while (check.retry);
 	}
 
+	/* wake up threads blocking for 'queue_*()' or 'write()' */
+	Libc::resume_all();
+
 	if ((out_result != Result::READ_OK) ||
 	    (out_count < sizeof(Dirent))) {
 		return 0;
@@ -979,6 +992,9 @@ int Libc::Vfs_plugin::symlink(const char *oldpath, const char *newpath)
 		Libc::suspend(check);
 	} while (check.retry);
 
+	/* wake up threads blocking for 'queue_*()' or 'write()' */
+	Libc::resume_all();
+
 	_vfs_sync(handle);
 	VFS_THREAD_SAFE(handle->close());
 
@@ -1078,6 +1094,9 @@ ssize_t Libc::Vfs_plugin::readlink(const char *path, char *buf, ::size_t buf_siz
 		} while (check.retry);
 	}
 
+	/* wake up threads blocking for 'queue_*()' or 'write()' */
+	Libc::resume_all();
+
 	switch (out_result) {
 	case Result::READ_ERR_AGAIN:       return Errno(EAGAIN);
 	case Result::READ_ERR_WOULD_BLOCK: return Errno(EWOULDBLOCK);

repos/os/include/vfs/file_io_service.h

@@ -47,8 +47,9 @@ struct Vfs::File_io_service : Interface
 	 ***********/
 
 	/*
-	 * Exception, thrown when 'alloc_packet()' or 'submit_packet()' failed in the
-	 * VFS plugin and the caller should wait for an IO response and try again.
+	 * Exception, thrown when, for example, 'alloc_packet()' or
+	 * 'submit_packet()' failed in the VFS plugin. The caller can try again
+	 * after a previous VFS request is completed.
 	 */
 	struct Insufficient_buffer { };
 
@@ -74,6 +75,9 @@ struct Vfs::File_io_service : Interface
 	 * Queue read operation
 	 *
 	 * \return false if queue is full
+	 *
+	 * If the queue is full, the caller can try again after a previous VFS
+	 * request is completed.
 	 */
 	virtual bool queue_read(Vfs_handle *, file_size)
 	{
@@ -177,6 +181,9 @@ struct Vfs::File_io_service : Interface
 	 * Queue sync operation
 	 *
 	 * \return false if queue is full
+	 *
+	 * If the queue is full, the caller can try again after a previous VFS
+	 * request is completed.
 	 */
 	virtual bool queue_sync(Vfs_handle *) { return true; }
 

repos/os/src/lib/vfs/fs_file_system.h

@@ -140,12 +140,6 @@ class Vfs::Fs_file_system : public File_system
 
 				source.release_packet(packet);
 
-				/*
-				 * Notify anyone who might have failed on
-				 * 'alloc_packet()' or 'submit_packet()'
-				 */
-				_io_handler.handle_io_response(nullptr);
-
 				return READ_OK;
 			}
 
@@ -222,12 +216,6 @@ class Vfs::Fs_file_system : public File_system
 
 				source.release_packet(packet);
 
-				/*
-				 * Notify anyone who might have failed on
-				 * 'alloc_packet()' or 'submit_packet()'
-				 */
-				_io_handler.handle_io_response(nullptr);
-
 				return SYNC_OK;
 			}
 		};

repos/ports/src/noux/rom_session_component.h

@@ -106,6 +106,11 @@ struct Noux::Vfs_dataspace
 					read_context.vfs_io_waiter.wait_for_io();
 				}
 
+				/* wake up threads blocking for 'queue_*()' or 'write()' */
+				vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+					r.wakeup();
+				});
+
 				if (read_result != Vfs::File_io_service::READ_OK) {
 					Genode::error("Error reading dataspace from VFS");
 					rm.detach(addr);

repos/ports/src/noux/syscall.cc

@@ -103,11 +103,6 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				size_t   path_len  = strlen(_sysio.stat_in.path);
 				uint32_t path_hash = hash_path(_sysio.stat_in.path, path_len);
 
-				/* XXX: remove sync */
-
-				Registered_no_delete<Vfs_io_waiter>
-					vfs_io_waiter(_vfs_io_waiter_registry);
-
 				Vfs::Directory_service::Stat stat_out;
 				_sysio.error.stat = _root_dir.stat(_sysio.stat_in.path, stat_out);
 
@@ -668,6 +663,11 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				read_context.vfs_io_waiter.wait_for_io();
 			}
 
+			/* wake up threads blocking for 'queue_*()' or 'write()' */
+			_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+				r.wakeup();
+			});
+
 			symlink_handle->ds().close(symlink_handle);
 
 			_sysio.readlink_out.count = out_count;
@@ -766,6 +766,11 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				}
 			}
 
+			/* wake up threads blocking for 'queue_*()' or 'write()' */
+			_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+				r.wakeup();
+			});
+
 			if (out_count != count) {
 				_sysio.error.symlink = Sysio::SYMLINK_ERR_NAME_TOO_LONG;
 				result = false;
@@ -782,6 +787,11 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				   Vfs::File_io_service::SYNC_QUEUED)
 				sync_context.vfs_io_waiter.wait_for_io();
 
+			/* wake up threads blocking for 'queue_*()' or 'write()' */
+			_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+				r.wakeup();
+			});
+
 			symlink_handle->ds().close(symlink_handle);
 
 			break;
@@ -916,6 +926,11 @@ bool Noux::Child::syscall(Noux::Session::Syscall sc)
 				   Vfs::File_io_service::SYNC_QUEUED)
 					sync_context.vfs_io_waiter.wait_for_io();
 
+				/* wake up threads blocking for 'queue_*()' or 'write()' */
+				_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+					r.wakeup();
+				});
+
 				sync_handle->ds().close(sync_handle);
 
 				break;

repos/ports/src/noux/vfs_io_channel.h

@@ -79,6 +79,11 @@ struct Noux::Vfs_io_channel : Io_channel
 
 		while (_fh->fs().complete_sync(_fh) == Vfs::File_io_service::SYNC_QUEUED)
 			vfs_io_waiter.wait_for_io();
+
+		/* wake up threads blocking for 'queue_*()' or 'write()' */
+		_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+			r.wakeup();
+		});
 	}
 
 	Vfs_io_channel(char const *path, char const *leaf_path,
@@ -123,6 +128,11 @@ struct Noux::Vfs_io_channel : Io_channel
 			}
 		}
 
+		/* wake up threads blocking for 'queue_*()' or 'write()' */
+		_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+			r.wakeup();
+		});
+
 		if (sysio.error.write != Vfs::File_io_service::WRITE_OK)
 			return false;
 
@@ -160,6 +170,11 @@ struct Noux::Vfs_io_channel : Io_channel
 			vfs_io_waiter.wait_for_io();
 		}
 
+		/* wake up threads blocking for 'queue_*()' or 'write()' */
+		_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+			r.wakeup();
+		});
+	
 		if (sysio.error.read != Vfs::File_io_service::READ_OK)
 			return false;
 
@@ -271,6 +286,11 @@ struct Noux::Vfs_io_channel : Io_channel
 			vfs_io_waiter.wait_for_io();
 		}
 
+		/* wake up threads blocking for 'queue_*()' or 'write()' */
+		_vfs_io_waiter_registry.for_each([] (Vfs_io_waiter &r) {
+			r.wakeup();
+		});
+
 		if ((read_result != Vfs::File_io_service::READ_OK) ||
 		    (out_count != sizeof(dirent))) {
 		    dirent = Vfs::Directory_service::Dirent();