blk_cache: fix deadlock in allocator hierarchy

This commit generalizes the bit array in 'base/util/bit_array.h',
so that it can be used in a statically, when the array size is known
at compile time, or dynamically. It uses the dynamic approach of the
bit array for a more generalized version of the packet allocator,
formerly only used by NIC session clients. The more generic packet
allocator is used by the block cache to circumvent the allocation
deadlock described in issue #1059.

Fixes #1059

base/include/util/bit_array.h

@@ -15,31 +15,32 @@
 #ifndef _INCLUDE__UTIL__BIT_ARRAY_H_
 #define _INCLUDE__UTIL__BIT_ARRAY_H_
 
+#include <util/string.h>
 #include <base/exception.h>
 #include <base/stdint.h>
 
 namespace Genode {
 
-	template <unsigned BITS>
+	class Bit_array_base
-	class Bit_array
 	{
 		public:
 
+			class Invalid_bit_count    : public Exception {};
 			class Invalid_index_access : public Exception {};
 			class Invalid_clear        : public Exception {};
 			class Invalid_set          : public Exception {};
 
-		private:
+		protected:
 
 			static constexpr size_t _BITS_PER_BYTE = 8UL;
 			static constexpr size_t _BITS_PER_WORD = sizeof(addr_t) *
 			                                        _BITS_PER_BYTE;
-			static constexpr size_t _WORDS         = BITS / _BITS_PER_WORD;
 
-			static_assert(BITS % _BITS_PER_WORD == 0,
+		private:
-			              "Count of bits need to be word aligned!");
 
-			addr_t _words[_WORDS];
+			unsigned _bit_cnt;
+			unsigned _word_cnt;
+			addr_t  *_words;
 
 			addr_t _word(addr_t index) const {
 				return index / _BITS_PER_WORD; }
@@ -47,9 +48,9 @@ namespace Genode {
 			void _check_range(addr_t const index,
 			                  addr_t const width) const
 			{
-				if ((index >= _WORDS * _BITS_PER_WORD) ||
+				if ((index >= _word_cnt * _BITS_PER_WORD) ||
-				    width > _WORDS * _BITS_PER_WORD ||
+				    width > _word_cnt * _BITS_PER_WORD ||
-				    _WORDS * _BITS_PER_WORD - width < index)
+				    _word_cnt * _BITS_PER_WORD - width < index)
 					throw Invalid_index_access();
 			}
 
@@ -91,7 +92,15 @@ namespace Genode {
 
 		public:
 
-			Bit_array() { memset(&_words, 0, sizeof(_words)); }
+			Bit_array_base(unsigned bits, addr_t *addr)
+			: _bit_cnt(bits),
+			  _word_cnt(_bit_cnt / _BITS_PER_WORD),
+			  _words(addr)
+			{
+				if (bits % _BITS_PER_WORD) throw Invalid_bit_count();
+
+				memset(_words, 0, sizeof(addr_t)*_word_cnt);
+			}
 
 			/**
 			 * Return true if at least one bit is set between
@@ -120,5 +129,23 @@ namespace Genode {
 				_set(index, width, true); }
 	};
 
+
+	template <unsigned BITS>
+	class Bit_array : public Bit_array_base
+	{
+		private:
+
+			static constexpr size_t _WORDS = BITS / _BITS_PER_WORD;
+
+			static_assert(BITS % _BITS_PER_WORD == 0,
+			              "Count of bits need to be word aligned!");
+
+			addr_t _array[_WORDS];
+
+		public:
+
+			Bit_array() : Bit_array_base(BITS, _array) { }
+	};
+
 }
 #endif /* _INCLUDE__UTIL__BIT_ARRAY_H_ */

os/include/nic/packet_allocator.h

@@ -16,22 +16,15 @@
 #ifndef _INCLUDE__NIC__PACKET_ALLOCATOR__
 #define _INCLUDE__NIC__PACKET_ALLOCATOR__
 
-#include <base/allocator.h>
+#include <os/packet_allocator.h>
-#include <util/string.h>
 
 namespace Nic {
 
-	class Packet_allocator : public Genode::Range_allocator
+	/**
+	 * Packet allocator used for packet streaming in nic sessions.
+	 */
+	class Packet_allocator : public Genode::Packet_allocator
 	{
-		private:
-
-			Genode::Allocator *_md_alloc;   /* meta-data allocator */
-			unsigned           _block_size; /* network packet size */
-			Genode::addr_t     _base;       /* allocation base */
-			unsigned          *_free;       /* free list */
-			int                _curr_idx;   /* current index in free list */
-			int                _count;      /* number of elements */
-
 		public:
 
 			enum { DEFAULT_PACKET_SIZE = 1600 };
@@ -39,92 +32,10 @@ namespace Nic {
 			/**
 			 * Constructor
 			 *
-			 * \param md_alloc       Meta-data allocator
+			 * \param md_alloc  Meta-data allocator
-			 * \param block_size     Size of network packet in stream
 			 */
-			Packet_allocator(Genode::Allocator *md_alloc,
+			Packet_allocator(Genode::Allocator *md_alloc)
-			                 unsigned block_size = DEFAULT_PACKET_SIZE)
+			: Genode::Packet_allocator(md_alloc, DEFAULT_PACKET_SIZE) {}
-			: _md_alloc(md_alloc), _block_size(block_size), _base(0), _free(0),
-			  _curr_idx(0), _count(0)
-			{}
-
-			~Packet_allocator()
-			{
-				if (_free)
-					_md_alloc->free(_free, sizeof(unsigned) * (_count / 32));
-			}
-
-
-			/*******************************
-			 ** Range-allocator interface **
-			 *******************************/
-
-			int add_range(Genode::addr_t base, Genode::size_t size)
-			{
-				if (_count)
-					return -1;
-
-				_base  = base;
-				_count = size / _block_size;
-				_free  = (unsigned *)_md_alloc->alloc(sizeof(unsigned) * (_count / 32));
-
-				Genode::memset(_free, 0xff, sizeof(unsigned) * (_count / 32));
-				return 0;
-			}
-
-			Alloc_return alloc_aligned(Genode::size_t size, void **out_addr, int) {
-				return alloc(size, out_addr) ? Alloc_return::OK
-				                             : Alloc_return::RANGE_CONFLICT; }
-
-			bool alloc(Genode::size_t size, void **out_addr)
-			{ 
-				int free_cnt = _count / 32;
-
-				for (register int i = 0; i < free_cnt; i++) {
-
-					if (_free[_curr_idx] != 0) {
-						unsigned msb = Genode::log2(_free[_curr_idx]);
-						int elem_idx = (_curr_idx * 32) + msb;
-
-						if (elem_idx < _count) {
-							_free[_curr_idx] ^= (1 << msb);
-							*out_addr = reinterpret_cast<void *>(_base + (_block_size * elem_idx));
-							return true;
-						}
-					}
-
-					_curr_idx = (_curr_idx + 1) % free_cnt;
-				}
-				return false;
-			}
-
-			void free(void *addr)
-			{
-				Genode::addr_t a = reinterpret_cast<Genode::addr_t>(addr);
-
-				int elem_idx = (a - _base) / _block_size;
-				if (elem_idx >= _count)
-					return;
-
-				_curr_idx = elem_idx / 32;
-				_free[_curr_idx] |= (1 << (elem_idx % 32));
-			}
-
-			void free(void *addr, Genode::size_t) { free(addr); }
-
-			bool need_size_for_free() const override { return false; }
-
-
-			/*********************
-			 ** Dummy functions **
-			 *********************/
-
-			Genode::size_t overhead(Genode::size_t) {  return 0;}
-			int remove_range(Genode::addr_t, Genode::size_t) { return 0;}
-			Genode::size_t avail() { return 0; }
-			bool valid_addr(Genode::addr_t) { return 0; }
-			Alloc_return alloc_addr(Genode::size_t, Genode::addr_t) {
-				return Alloc_return(Alloc_return::OUT_OF_METADATA); }
 	};
 };
 

os/include/os/packet_allocator.h

@@ -0,0 +1,148 @@
+/*
+ * \brief  Fast-bitmap allocator for packet streams
+ * \author Sebastian Sumpf
+ * \author Stefan Kalkowski
+ * \date   2012-07-30
+ */
+
+/*
+ * Copyright (C) 2012-2013 Genode Labs GmbH
+ *
+ * This file is part of the Genode OS framework, which is distributed
+ * under the terms of the GNU General Public License version 2.
+ */
+
+#ifndef _INCLUDE__OS__PACKET_ALLOCATOR__
+#define _INCLUDE__OS__PACKET_ALLOCATOR__
+
+#include <base/allocator.h>
+#include <util/bit_array.h>
+
+namespace Genode {
+	class Packet_allocator;
+}
+
+
+/**
+ * This allocator is designed to be used as packet allocator for the
+ * packet stream interface. It uses a minimal block size, which is the
+ * granularity packets will be allocated with. As backend, it uses a
+ * simple bit array to manage free, and allocated blocks.
+ */
+class Genode::Packet_allocator : public Genode::Range_allocator
+{
+	private:
+
+		Allocator      *_md_alloc;   /* meta-data allocator                 */
+		size_t          _block_size; /* granularity of packet allocations   */
+		void           *_bits;       /* memory chunk containing the bits    */
+		Bit_array_base *_array;      /* bit array managing available blocks */
+		addr_t          _base;       /* allocation base                     */
+		addr_t          _next;       /* next free bit index                 */
+
+		/*
+		 * Returns the count of blocks fitting the given size
+		 *
+		 * The block count returned is aligned to the bit count
+		 * of a machine word to fit the needs of the used bit array.
+		 */
+		inline size_t _block_cnt(size_t bytes)
+		{
+			bytes /= _block_size;
+			return bytes - (bytes % (sizeof(addr_t)*8));
+		}
+
+	public:
+
+		/**
+		 * Constructor
+		 *
+		 * \param md_alloc       Meta-data allocator
+		 * \param block_size     Granularity of packets in stream
+		 */
+		Packet_allocator(Allocator *md_alloc, size_t block_size)
+		: _md_alloc(md_alloc), _block_size(block_size), _bits(0),
+		  _array(nullptr), _base(0) {}
+
+
+		/*******************************
+		 ** Range-allocator interface **
+		 *******************************/
+
+		int add_range(addr_t base, size_t size)
+		{
+			if (_base || _array) return -1;
+
+			_base  = base;
+			_bits  = _md_alloc->alloc(_block_cnt(size)/8);
+			_array = new (_md_alloc) Bit_array_base(_block_cnt(size),
+			                                        (addr_t*)_bits);
+			return 0;
+		}
+
+		int remove_range(addr_t base, size_t size)
+		{
+			if (_base != base) return -1;
+
+			if (_array) destroy(_md_alloc, _array);
+			if (_bits)  _md_alloc->free(_bits, _block_cnt(size)/8);
+			return 0;
+		}
+
+		Alloc_return alloc_aligned(size_t size, void **out_addr, int) {
+			return alloc(size, out_addr) ? Alloc_return::OK
+			                             : Alloc_return::RANGE_CONFLICT; }
+
+		bool alloc(size_t size, void **out_addr)
+		{
+			addr_t const cnt = (size % _block_size) ? size / _block_size + 1
+			                                        : size / _block_size;
+			addr_t max = ~0UL;
+
+			do {
+				try {
+					/* throws exception if array is accessed outside bounds */
+					for (addr_t i = _next & ~(cnt - 1); i < max; i += cnt) {
+						if (_array->get(i, cnt))
+							continue;
+
+						_array->set(i, cnt);
+						_next = i + cnt;
+						*out_addr = reinterpret_cast<void *>(i * _block_size
+						                                     + _base);
+						return true;
+					}
+				} catch (typename Bit_array_base::Invalid_index_access) { }
+
+				max = _next;
+				_next = 0;
+
+			} while (max != 0);
+
+			return false;
+		}
+
+		void free(void *addr, size_t size)
+		{
+			addr_t i   = (((addr_t)addr) - _base) / _block_size;
+			size_t cnt = (size % _block_size) ? size / _block_size + 1
+			                                  : size / _block_size;
+			_array->clear(i, cnt);
+			_next = i;
+		}
+
+
+		/*********************
+		 ** Dummy functions **
+		 *********************/
+
+		bool need_size_for_free() const override { return false; }
+		void free(void *addr) { }
+		size_t overhead(size_t) {  return 0;}
+		size_t avail() { return 0; }
+		bool valid_addr(addr_t) { return 0; }
+		Alloc_return alloc_addr(size_t, addr_t) {
+			return Alloc_return(Alloc_return::OUT_OF_METADATA); }
+};
+
+#endif /* _INCLUDE__OS__PACKET_ALLOCATOR__ */

os/src/server/blk_cache/driver.h

@@ -14,6 +14,7 @@
 #include <base/printf.h>
 #include <block_session/connection.h>
 #include <block/component.h>
+#include <os/packet_allocator.h>
 
 #include "chunk.h"
 
@@ -117,7 +118,7 @@ class Driver : public Block::Driver
 
 		Genode::Tslab<Request, SLAB_SZ>   _r_slab;    /* slab for requests  */
 		Genode::List<Request>             _r_list;    /* list of requests   */
-		Genode::Allocator_avl             _alloc;     /* packet allocator   */
+		Genode::Packet_allocator          _alloc;     /* packet allocator   */
 		Block::Connection                 _blk;       /* backend device     */
 		Block::Session::Operations        _ops;       /* allowed operations */
 		Genode::size_t                    _blk_sz;    /* block size         */
@@ -255,7 +256,6 @@ class Driver : public Block::Driver
 			} catch(Genode::Allocator::Out_of_memory) {
 				if (p_to_dev.valid()) /* clean up */
 					_blk.tx()->release_packet(p_to_dev);
-				//TODO
 				throw Request_congestion();
 			}
 		}
@@ -332,7 +332,7 @@ class Driver : public Block::Driver
 		 */
 		Driver(Server::Entrypoint &ep)
 		: _r_slab(Genode::env()->heap()),
-		  _alloc(Genode::env()->heap()),
+		  _alloc(Genode::env()->heap(), CACHE_BLK_SIZE),
 		  _blk(&_alloc, Block::Session::TX_QUEUE_SIZE*CACHE_BLK_SIZE),
 		  _blk_sz(0),
 		  _blk_cnt(0),