In function Req_cache::remove on ports-foc/src/lib/l4lx/genode_block.cc file:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } *request = _cache[idx].req; // <--- if idx is -1, this negative index used for access _cache array elements _cache[idx].pkt = 0; _cache[idx].req = 0; }
should be:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } else { *request = _cache[idx].req; _cache[idx].pkt = 0; _cache[idx].req = 0; } }
Hello Ildar,
thank you for pointing to this. I wonder whether the error condition was met in some of your scenarios? Obviously that should never be the case. I think the best will be to change that snippet into an assertion statement.
Regards Stefan
On 16.10.2012 10:33, Ildar Ismagilov wrote:
In function Req_cache::remove on ports-foc/src/lib/l4lx/genode_block.cc file:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } *request = _cache[idx].req; // <--- if idx is -1, this negative index used for access _cache array elements _cache[idx].pkt = 0; _cache[idx].req = 0; }
should be:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } else { *request = _cache[idx].req; _cache[idx].pkt = 0; _cache[idx].req = 0; } }
2012/10/16 Ildar Ismagilov <devix84@...9...>:
In function Req_cache::remove on ports-foc/src/lib/l4lx/genode_block.cc file:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } *request = _cache[idx].req; // <--- if idx is -1, this negative index used for access _cache array elements _cache[idx].pkt = 0; _cache[idx].req = 0; }
should be:
void remove(void *packet, void **request) { int idx = _find(packet);
if (idx < 0) { *request = 0; PERR("Req cache entry not found!"); } else { *request = _cache[idx].req; _cache[idx].pkt = 0; _cache[idx].req = 0; } }
-- Best regards Ildar
Test case is intensive usage of block driver:
If "devices[idx]->cache()->insert(addr, req);" function called from genode_block_request when Req_cache::_cache array is full, the Req_cache::_find(0) return -1, and data don't added to Req_cache::_cache array. Next "devices[idx]->cache()->remove(session->tx()->packet_content(packet), &req)" function call from "genode_block_collect_responses" will delete data from Req_cache::_cache array but Req_cache::_find(packet) will return -1 because previous "Req_cache::insert" call not inserted item to Req_cache::_cache array and this bug appears.
Hi Ildar,
On 18.10.2012 11:29, Ildar wrote:
Test case is intensive usage of block driver:
If "devices[idx]->cache()->insert(addr, req);" function called from genode_block_request when Req_cache::_cache array is full, the Req_cache::_find(0) return -1, and data don't added to Req_cache::_cache array. Next "devices[idx]->cache()->remove(session->tx()->packet_content(packet), &req)" function call from "genode_block_collect_responses" will delete data from Req_cache::_cache array but Req_cache::_find(packet) will return -1 because previous "Req_cache::insert" call not inserted item to Req_cache::_cache array and this bug appears.
well that should never happen that the request cache is smaller than the request-queue size of the block-session. If we just try to cover the error case of insert/remove from the cache we'll get integrity problems in Linux because single blocks won't get read correctly, or won't get written. I've fixed that problem by always dimensioning the cache according to the block-session's queue size. Please refer to issue #426 in our issue tracker.
Btw. would you mind to report, and discuss such issues in our issue tracker. Of course, general discussions, design issues, and so on furthermore are best placed on the mailing list.
Regards Stefan
-
Ildar -
Ildar Ismagilov -
Stefan Kalkowski