Assert in packagefs/CachedDataReader.cpp

[miqlas]

Just came from the blue sky. This is hrev55094 x86_64. I was building qemu on SSD, extracting some tarballs on ramdisk and downloading libs in Web+. waddlesplash said it is bad and i should open a ticket, so here is it.

[madmax]

Very probably introduced in hrev55085.

If the page iterator does not return all the pages for the cache line and the ones returned fulfill the request (something I have not seen happen yet), the allocation is skipped, some pages in the line are NULL, _CachePages receives them later (it's called with th full line) and the assert is triggered.

The quick fix may be to just accept NULL pages in _CachePages, like they already are in _DiscardPages. Another possibility would be to pass the offset and length really used to fulfill the request. In the latter case, more pages may have been marked unused, should they also be marked cached again? Maybe we should mark them unused and later cached only if we can't skip the allocation?

The IOCache change would also be affected.

Maybe it's better to revert the change until someone with knowledge in vm and the caches used have a deep look at that and determine if it's worth it and the best path to follow?

[korli]

reverted in hrev55103