Opened 3 years ago

Last modified 3 years ago

#16954 new bug

Assert in packagefs/CachedDataReader.cpp

Reported by: miqlas Owned by: bonefish
Priority: normal Milestone: Unscheduled
Component: File Systems/packagefs Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Platform: x86-64

Description

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.

Attachments (1)

photo_2021-05-21_18-25-40.jpg (280.6 KB ) - added by miqlas 3 years ago.

Download all attachments as: .zip

Change History (4)

by miqlas, 3 years ago

comment:1 by madmax, 3 years ago

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?

comment:2 by korli, 3 years ago

reverted in hrev55103

comment:3 by waddlesplash, 3 years ago

Component: - GeneralFile Systems/packagefs
Owner: changed from nobody to bonefish
Note: See TracTickets for help on using tickets.