block_session: SYNC and TRIM as async operations

This patch removes the blocking Block::Session::sync RPC function and
adds the asynchronous operations SYNC and TRIM to the block session's
packet-stream interface.

Even though the patch adjusts all block components to the interface
change, the components keep the former blocking handling of sync
internally for now because of the design of the 'Block::Driver'
interface. This old interface is not worth changing. We should instead
migrate the block servers step by step to the new
'Block::Request_stream' API.

Fixes #3274

repos/dde_rump/src/lib/rump/io.cc

@@ -34,6 +34,15 @@ class Backend
 		Block::Session::Info  _info { _session.info() };
 		Genode::Lock          _session_lock;
 
+		void _sync()
+		{
+			using Block::Session;
+
+			Session::Tag const tag { 0 };
+			_session.tx()->submit_packet(Session::sync_all_packet_descriptor(_info, tag));
+			_session.tx()->get_acked_packet();
+		}
+
 	public:
 
 		uint64_t block_count() const { return _info.block_count; }
@@ -43,7 +52,7 @@ class Backend
 		void sync()
 		{
 			Genode::Lock::Guard guard(_session_lock);
-			_session.sync();
+			_sync();
 		}
 
 		bool submit(int op, int64_t offset, size_t length, void *data)
@@ -83,7 +92,7 @@ class Backend
 
 			/* sync request */
 			if (op & RUMPUSER_BIO_SYNC) {
-				_session.sync();
+				_sync();
 			}
 
 			return succeeded;

repos/libports/src/lib/fatfs/diskio_block.cc

@@ -61,9 +61,19 @@ extern "C" {
 		Info const info = Block::Connection::info();
 
 		using Block::Connection::tx;
-		using Block::Connection::sync;
 		using Block::Connection::alloc_packet;
 
+		void sync()
+		{
+			/*
+			 * We don't need to distinguish tags because there can only be one
+			 * outstanding request.
+			 */
+			Block::Session::Tag const tag { 0 };
+			tx()->submit_packet(sync_all_packet_descriptor(info, tag));
+			tx()->get_acked_packet();
+		}
+
 		Drive(Platform &platform, char const *label)
 		: Block::Connection(platform.env, &platform.tx_alloc, 128*1024, label)
 		{ }

repos/os/include/block/component.h

@@ -148,6 +148,24 @@ class Block::Session_component : public Block::Session_component_base,
 						              _p_to_handle);
 					break;
 
+				case Block::Packet_descriptor::SYNC:
+
+					/* perform (blocking) sync */
+					_driver.sync();
+
+					_p_to_handle.succeeded(true);
+					_ack_packet(_p_to_handle);
+					_p_to_handle = Packet_descriptor();
+					break;
+
+				case Block::Packet_descriptor::TRIM:
+
+					/* trim is a nop */
+					_p_to_handle.succeeded(true);
+					_ack_packet(_p_to_handle);
+					_p_to_handle = Packet_descriptor();
+					break;
+
 				default:
 					throw Driver::Io_error();
 				}
@@ -244,8 +262,6 @@ class Block::Session_component : public Block::Session_component_base,
 		 *******************************/
 
 		Info info() const override { return _driver.info(); }
-
-		void sync() override { _driver.sync(); }
 };
 
 

repos/os/include/block/request.h

@@ -30,7 +30,7 @@ namespace Block {
 
 struct Block::Operation
 {
-	enum class Type { INVALID = 0, READ = 1, WRITE = 2, SYNC = 3 };
+	enum class Type { INVALID = 0, READ = 1, WRITE = 2, SYNC = 3, TRIM = 4 };
 
 	Type           type;
 	block_number_t block_number;
@@ -38,7 +38,8 @@ struct Block::Operation
 
 	bool valid() const
 	{
-		return type == Type::READ || type == Type::WRITE || type == Type::SYNC;
+		return type == Type::READ || type == Type::WRITE
+		    || type == Type::SYNC || type == Type::TRIM;
 	}
 };
 

repos/os/include/block/request_stream.h

@@ -195,6 +195,8 @@ class Block::Request_stream : Genode::Noncopyable
 					switch (op) {
 					case Packet_descriptor::READ:  return Operation::Type::READ;
 					case Packet_descriptor::WRITE: return Operation::Type::WRITE;
+					case Packet_descriptor::SYNC:  return Operation::Type::SYNC;
+					case Packet_descriptor::TRIM:  return Operation::Type::TRIM;
 					case Packet_descriptor::END:   return Operation::Type::INVALID;
 					};
 					return Operation::Type::INVALID;
@@ -286,6 +288,7 @@ class Block::Request_stream : Genode::Noncopyable
 						case Operation::Type::READ:    return Packet_descriptor::READ;
 						case Operation::Type::WRITE:   return Packet_descriptor::WRITE;
 						case Operation::Type::SYNC:    return Packet_descriptor::END;
+						case Operation::Type::TRIM:    return Packet_descriptor::TRIM;
 						case Operation::Type::INVALID: return Packet_descriptor::END;
 						};
 						return Packet_descriptor::END;

repos/os/include/block_session/block_session.h

@@ -31,7 +31,7 @@ namespace Block {
 
 
 /**
- * Representation of an block-operation request
+ * Representation of a block-operation request
  *
  * The data associated with the 'Packet_descriptor' is either
  * the data read from or written to the block indicated by
@@ -41,7 +41,7 @@ class Block::Packet_descriptor : public Genode::Packet_descriptor
 {
 	public:
 
-		enum Opcode    { READ, WRITE, END };
+		enum Opcode { READ, WRITE, SYNC, TRIM, END };
 
 		/*
 		 * Alignment used when allocating a packet directly via the 'tx'
@@ -145,11 +145,6 @@ struct Block::Session : public Genode::Session
 	 */
 	virtual Info info() const = 0;
 
-	/**
-	 * Synchronize with block device, like ensuring data to be written
-	 */
-	virtual void sync() = 0;
-
 	/**
 	 * Request packet-transmission channel
 	 */
@@ -165,6 +160,16 @@ struct Block::Session : public Genode::Session
 	 */
 	virtual Genode::Capability<Tx> tx_cap() = 0;
 
+	/**
+	 * Return packet descriptor for syncing the entire block session
+	 */
+	static Packet_descriptor sync_all_packet_descriptor(Info const &info, Tag tag)
+	{
+		return Packet_descriptor(Packet_descriptor(0UL, 0UL),
+		                         Packet_descriptor::SYNC,
+		                         0UL, info.block_count, tag);
+	}
+
 
 	/*******************
 	 ** RPC interface **
@@ -172,8 +177,7 @@ struct Block::Session : public Genode::Session
 
 	GENODE_RPC(Rpc_info, Info, info);
 	GENODE_RPC(Rpc_tx_cap, Genode::Capability<Tx>, tx_cap);
-	GENODE_RPC(Rpc_sync, void, sync);
+	GENODE_RPC_INTERFACE(Rpc_info, Rpc_tx_cap);
-	GENODE_RPC_INTERFACE(Rpc_info, Rpc_tx_cap, Rpc_sync);
 };
 
 #endif /* _INCLUDE__BLOCK_SESSION__BLOCK_SESSION_H_ */

repos/os/include/block_session/client.h

@@ -57,8 +57,6 @@ class Block::Session_client : public Genode::Rpc_client<Session>
 
 		Tx::Source *tx() override { return _tx.source(); }
 
-		void sync() override { call<Rpc_sync>(); }
-
 		Genode::Capability<Tx> tx_cap() override { return call<Rpc_tx_cap>(); }
 
 		/**

repos/os/src/server/part_block/component.h

@@ -89,27 +89,55 @@ class Block::Session_component : public  Block::Session_rpc_object,
 				return;
 			}
 
-			bool write   = _p_to_handle.operation() == Packet_descriptor::WRITE;
+			sector_t const off = _p_to_handle.block_number() + _partition->lba;
-			sector_t off = _p_to_handle.block_number() + _partition->lba;
+			size_t   const cnt = _p_to_handle.block_count();
-			size_t cnt   = _p_to_handle.block_count();
 
-			if (write && !_writeable) {
+			auto perform_io = [&] ()
-				_ack_packet(_p_to_handle);
+			{
-				return;
+				bool const write =
-			}
+					(_p_to_handle.operation() == Packet_descriptor::WRITE);
 
-			try {
+				try {
-				_driver.io(write, off, cnt,
+					_driver.io(write, off, cnt,
-				           tx_sink()->packet_content(_p_to_handle),
+					           tx_sink()->packet_content(_p_to_handle),
-				           *this, _p_to_handle);
+					           *this, _p_to_handle);
-			} catch (Block::Session::Tx::Source::Packet_alloc_failed) {
+				} catch (Block::Session::Tx::Source::Packet_alloc_failed) {
-				if (!_req_queue_full) {
+					if (!_req_queue_full) {
-					_req_queue_full = true;
+						_req_queue_full = true;
-					Session_component::wait_queue().insert(this);
+						Session_component::wait_queue().insert(this);
+					}
+				} catch (Genode::Packet_descriptor::Invalid_packet) {
+					Genode::error("dropping invalid Block packet");
+					_p_to_handle = Packet_descriptor();
 				}
-			} catch (Genode::Packet_descriptor::Invalid_packet) {
+			};
-				Genode::error("dropping invalid Block packet");
+
-				_p_to_handle = Packet_descriptor();
+			switch (_p_to_handle.operation()) {
+
+			case Packet_descriptor::READ:
+				perform_io();
+				break;
+
+			case Packet_descriptor::WRITE:
+
+				if (!_writeable) {
+					_ack_packet(_p_to_handle);
+					return;
+				}
+
+				perform_io();
+				break;
+
+			case Packet_descriptor::SYNC:
+				_driver.sync_all(*this, _p_to_handle);
+				break;
+
+			case Packet_descriptor::TRIM:
+			case Packet_descriptor::END:
+
+				_p_to_handle.succeeded(true);
+				_ack_packet(_p_to_handle);
+				break;
 			}
 		}
 
@@ -223,8 +251,6 @@ class Block::Session_component : public  Block::Session_rpc_object,
 			              .align_log2  = Genode::log2(_driver.blk_size()),
 			              .writeable   = _writeable && _driver.writeable() };
 		}
-
-		void sync() override { _driver.session().sync(); }
 };
 
 

repos/os/src/server/part_block/driver.h

@@ -37,9 +37,7 @@ struct Block::Block_dispatcher : Genode::Interface
 bool operator== (const Block::Packet_descriptor& p1,
                  const Block::Packet_descriptor& p2)
 {
-	return p1.operation()    == p2.operation()    &&
+	return p1.tag().value == p2.tag().value;
-	       p1.block_number() == p2.block_number() &&
-	       p1.block_count()  == p2.block_count();
 }
 
 
@@ -58,13 +56,13 @@ class Block::Driver
 		public:
 
 			Request(Block_dispatcher &d,
-			        Packet_descriptor &cli,
+			        Packet_descriptor const &cli,
-			        Packet_descriptor &srv)
+			        Packet_descriptor const &srv)
 			: _dispatcher(d), _cli(cli), _srv(srv) {}
 
 			bool handle(Packet_descriptor& reply)
 			{
-				bool ret =  reply == _srv;
+				bool ret = (reply == _srv);
 				if (ret) _dispatcher.dispatch(_cli, reply);
 				return ret;
 			}
@@ -84,6 +82,7 @@ class Block::Driver
 		Block::Session::Info     const _info { _session.info() };
 		Genode::Signal_handler<Driver> _source_ack;
 		Genode::Signal_handler<Driver> _source_submit;
+		unsigned long                  _tag_cnt { 0 };
 
 		void _ready_to_submit();
 
@@ -105,6 +104,17 @@ class Block::Driver
 			_ready_to_submit();
 		}
 
+		Block::Session::Tag _alloc_tag()
+		{
+			/*
+			 * The wrapping of '_tag_cnt' is no problem because the number
+			 * of consecutive outstanding requests is much lower than the
+			 * value range of tags.
+			 */
+			_tag_cnt++;
+			return Block::Session::Tag { _tag_cnt };
+		}
+
 	public:
 
 		Driver(Genode::Env &env, Genode::Heap &heap)
@@ -140,7 +150,7 @@ class Block::Driver
 			    : Block::Packet_descriptor::READ;
 			Genode::size_t const size = _info.block_size * cnt;
 			Packet_descriptor p(_session.alloc_packet(size),
-			                    op,  nr, cnt);
+			                    op,  nr, cnt, _alloc_tag());
 			Request *r = new (&_r_slab) Request(dispatcher, cli, p);
 			_r_list.insert(r);
 
@@ -151,6 +161,19 @@ class Block::Driver
 			_session.tx()->submit_packet(p);
 		}
 
+		void sync_all(Block_dispatcher &dispatcher, Packet_descriptor &cli)
+		{
+			if (!_session.tx()->ready_to_submit())
+				throw Block::Session::Tx::Source::Packet_alloc_failed();
+
+			Packet_descriptor const p =
+				Block::Session::sync_all_packet_descriptor(_info, _alloc_tag());
+
+			_r_list.insert(new (&_r_slab) Request(dispatcher, cli, p));
+
+			_session.tx()->submit_packet(p);
+		}
+
 		void remove_dispatcher(Block_dispatcher &dispatcher)
 		{
 			for (Request *r = _r_list.first(); r;) {

repos/os/src/test/block_request_stream/main.cc

@@ -60,8 +60,6 @@ struct Test::Block_session_component : Rpc_object<Block::Session>,
 
 	Info info() const override { return Request_stream::info(); }
 
-	void sync() override { }
-
 	Capability<Tx> tx_cap() override { return Request_stream::tx_cap(); }
 };