Revert "libdrgn: add last virtual address hint to page table iterator"

This reverts commit 747e028 (except for
the test improvements). Peter Collingbourne noticed that the change I
used to test the performance of reading a single PTE at a time [1]
didn't cache higher level entries. Keeping that caching makes the
regression I was worried about negligible. So, there's no reason to add
the extra complexity of the hint.

1: #312 (comment)

Signed-off-by: Omar Sandoval <osandov@osandov.com>

libdrgn/arch_x86_64.c

@@ -545,11 +545,7 @@ linux_kernel_live_direct_mapping_fallback_x86_64(struct drgn_program *prog,
 
 struct pgtable_iterator_x86_64 {
 	struct pgtable_iterator it;
-	// Index of next page table entry to use in a level.
 	uint16_t index[5];
-	// Index of end of cached entries in a level (i.e., the first index
-	// greater than index[level] that isn't cached).
-	uint16_t cached_index[5];
 	uint64_t table[5][512];
 };
 
@@ -575,8 +571,7 @@ linux_kernel_pgtable_iterator_init_x86_64(struct drgn_program *prog,
 {
 	struct pgtable_iterator_x86_64 *it =
 		container_of(_it, struct pgtable_iterator_x86_64, it);
-	memset(it->index, 0, sizeof(it->index));
-	memset(it->cached_index, 0, sizeof(it->cached_index));
+	memset(it->index, 0xff, sizeof(it->index));
 }
 
 static struct drgn_error *
@@ -614,7 +609,7 @@ linux_kernel_pgtable_iterator_next_x86_64(struct drgn_program *prog,
 	// Find the lowest level with cached entries.
 	int level;
 	for (level = 0; level < levels; level++) {
-		if (it->index[level] < it->cached_index[level])
+		if (it->index[level] < array_size(it->table[level]))
 			break;
 	}
 	// For every level below that, refill the cache/return pages.
@@ -643,26 +638,19 @@ linux_kernel_pgtable_iterator_next_x86_64(struct drgn_program *prog,
 			}
 			table_physical = true;
 		}
-		uint64_t index = (virt_addr >>
-				  (PAGE_SHIFT + PGTABLE_SHIFT * (level - 1)));
-		size_t num_to_cache = 1;
-		if (it->it.last_virt_addr_hint > virt_addr) {
-			uint64_t last_index = (it->it.last_virt_addr_hint >>
-					       (PAGE_SHIFT + PGTABLE_SHIFT * (level - 1)));
-			num_to_cache = min(last_index - index + 1,
-					   array_size(it->table[level - 1])
-					   - (index & PGTABLE_MASK));
-		}
-		index &= PGTABLE_MASK;
+		uint16_t index = ((virt_addr >>
+				  (PAGE_SHIFT + PGTABLE_SHIFT * (level - 1)))
+				  & PGTABLE_MASK);
+		// It's only marginally more expensive to read 4096 bytes than 8
+		// bytes, so we always read to the end of the table.
 		err = drgn_program_read_memory(prog,
 					       &it->table[level - 1][index],
 					       table + 8 * index,
-					       num_to_cache * 8,
+					       sizeof(it->table[0]) - 8 * index,
 					       table_physical);
 		if (err)
 			return err;
 		it->index[level - 1] = index;
-		it->cached_index[level - 1] = index + num_to_cache;
 	}
 }
 

libdrgn/linux_kernel_helpers.c

@@ -18,7 +18,7 @@ static void end_virtual_address_translation(struct drgn_program *prog)
 
 static struct drgn_error *
 begin_virtual_address_translation(struct drgn_program *prog, uint64_t pgtable,
-				  uint64_t virt_addr, uint64_t count_hint)
+				  uint64_t virt_addr)
 {
 	struct drgn_error *err;
 
@@ -54,7 +54,6 @@ begin_virtual_address_translation(struct drgn_program *prog, uint64_t pgtable,
 	}
 	prog->pgtable_it->pgtable = pgtable;
 	prog->pgtable_it->virt_addr = virt_addr;
-	prog->pgtable_it->last_virt_addr_hint = virt_addr + count_hint - 1;
 	prog->platform.arch->linux_kernel_pgtable_iterator_init(prog, prog->pgtable_it);
 	return NULL;
 
@@ -108,7 +107,7 @@ struct drgn_error *linux_helper_direct_mapping_offset(struct drgn_program *prog,
 
 	err = begin_virtual_address_translation(prog,
 						prog->vmcoreinfo.swapper_pg_dir,
-						virt_addr, 1);
+						virt_addr);
 	if (err)
 		return err;
 	uint64_t start_virt_addr, start_phys_addr;
@@ -138,8 +137,7 @@ struct drgn_error *linux_helper_read_vm(struct drgn_program *prog,
 {
 	struct drgn_error *err;
 
-	err = begin_virtual_address_translation(prog, pgtable, virt_addr,
-						count);
+	err = begin_virtual_address_translation(prog, pgtable, virt_addr);
 	if (err)
 		return err;
 	if (!count) {
@@ -198,7 +196,7 @@ struct drgn_error *linux_helper_follow_phys(struct drgn_program *prog,
 {
 	struct drgn_error *err;
 
-	err = begin_virtual_address_translation(prog, pgtable, virt_addr, 1);
+	err = begin_virtual_address_translation(prog, pgtable, virt_addr);
 	if (err)
 		return err;
 

libdrgn/platform.h

@@ -120,15 +120,6 @@ struct pgtable_iterator {
 	uint64_t pgtable;
 	/** Current virtual address to translate. */
 	uint64_t virt_addr;
-	/**
-	 * Last virtual address expected to be translated (inclusive).
-	 *
-	 * This is a hint; it may be less than or greater than the actual last
-	 * address that is translated. Page table iterator implementations
-	 * should optimize for the case that it is correct (e.g., by reading
-	 * ahead additional page table entries).
-	 */
-	uint64_t last_virt_addr_hint;
 };
 
 /**