From 746011ee2858f1e118926d80c3094819096afb2a Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Wed, 12 Feb 2014 13:59:41 +0100 Subject: [PATCH] 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 | 49 +++++++--- os/include/nic/packet_allocator.h | 105 ++------------------- os/include/os/packet_allocator.h | 148 ++++++++++++++++++++++++++++++ os/src/server/blk_cache/driver.h | 6 +- 4 files changed, 197 insertions(+), 111 deletions(-) create mode 100644 os/include/os/packet_allocator.h diff --git a/base/include/util/bit_array.h b/base/include/util/bit_array.h index e49a466f77..71599fa6ae 100644 --- a/base/include/util/bit_array.h +++ b/base/include/util/bit_array.h @@ -15,31 +15,32 @@ #ifndef _INCLUDE__UTIL__BIT_ARRAY_H_ #define _INCLUDE__UTIL__BIT_ARRAY_H_ +#include #include #include namespace Genode { - template - class Bit_array + class Bit_array_base { 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, - "Count of bits need to be word aligned!"); + private: - 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) || - width > _WORDS * _BITS_PER_WORD || - _WORDS * _BITS_PER_WORD - width < index) + if ((index >= _word_cnt * _BITS_PER_WORD) || + width > _word_cnt * _BITS_PER_WORD || + _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 + 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_ */ diff --git a/os/include/nic/packet_allocator.h b/os/include/nic/packet_allocator.h index 4f2c13166b..1b17e2b9c7 100644 --- a/os/include/nic/packet_allocator.h +++ b/os/include/nic/packet_allocator.h @@ -16,22 +16,15 @@ #ifndef _INCLUDE__NIC__PACKET_ALLOCATOR__ #define _INCLUDE__NIC__PACKET_ALLOCATOR__ -#include -#include +#include 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 block_size Size of network packet in stream + * \param md_alloc Meta-data allocator */ - Packet_allocator(Genode::Allocator *md_alloc, - unsigned block_size = 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(_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(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); } + Packet_allocator(Genode::Allocator *md_alloc) + : Genode::Packet_allocator(md_alloc, DEFAULT_PACKET_SIZE) {} }; }; diff --git a/os/include/os/packet_allocator.h b/os/include/os/packet_allocator.h new file mode 100644 index 0000000000..5745d8430b --- /dev/null +++ b/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 +#include + +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(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__ */ diff --git a/os/src/server/blk_cache/driver.h b/os/src/server/blk_cache/driver.h index f9fd13e8ed..53e9d4f61c 100644 --- a/os/src/server/blk_cache/driver.h +++ b/os/src/server/blk_cache/driver.h @@ -14,6 +14,7 @@ #include #include #include +#include #include "chunk.h" @@ -117,7 +118,7 @@ class Driver : public Block::Driver Genode::Tslab _r_slab; /* slab for requests */ Genode::List _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),