https://bugs.kde.org/show_bug.cgi?id=514297
--- Comment #26 from Mark Wielaard <[email protected]> --- (In reply to mcermak from comment #23) > > Note that significant effort was spent to make is_valid_for fast. > > ... > > So, ideally, checking for guard page should be fast either > > I've reimplemented the bookkeeping mechanism here to make it perform better: > https://sourceware.org/git/?p=valgrind.git;a=blob;f=coregrind/m_aspacemgr/ > aspacemgr-linux.c;h=7b449fd7a2a9204ffd6da7b9ca81e1847e08ef24; > hb=8727ba9b9edc9ebb053bb63e8eb8c8c49287f77a#l1078 > > The new implementation doesn't check /proc/self/pagemap at all by default. > Instead it has a new array guardpages[VG_N_THREADS] keeping an ordered list > of start addresses of madvise guard pages. The array is defined together > with a description explaining how its max size was set. Function > is_guarded() tries to quickly answer the question whether an address belongs > to a guard page. Expecting VG_N_THREADS of (max) guard pages seems like a good intuition. But maybe we want to introduce a separate constant/command line flag. I would suggest using VG(clo_max_guard_pages) and VG_N_GUARDS, which would default to VG_N_THREADS/VG(clo_max_threads) if not given. Do note that VG_N_THREADS (and VG_N_GUARDS) are not constants (you do define VG_N_THREADS now as constant). So the guardpages array should be allocated dynamically, either on am_startup or on first use (like the cache in find_nsegment_idx). is_guarded does a binary search and guard_page_install/guard_page_remove keep the array sorted (on page boundary). Nice. I wonder if you could also use a binary search in the install/remove case and move all addresses down/up. Might be slightly more efficient. But installs/removes are probably very infrequent. guard_page_remove allows for the address not to be found. But is that really possible? Does madvise (addr, size, MADV_GUARD_REMOVE) succeed or fail if the guard pages weren't installed first? If it fails then then guard_page_remove should probably assert when called on a (page boundary) address that cannot be found. But if that is legal and the syscall succeeds (without actually removing anything) then the code as written looks correct. -- You are receiving this mail because: You are watching all bug changes.
