NuKVM/memtest86plus
3
0
Fork 0
mirror of https://github.com/memtest86plus/memtest86plus.git synced 2025-02-25 18:55:23 -06:00
Code Issues Packages Projects Releases Wiki Activity

Change how BadRAM patterns are aggregated to minimize the number of covered addresses (#178)

Browse Source 
* BadRAM: Rename pattern -> patterns

* BadRAM: Refactor COMBINE_MASK and add clarifying comment

* BadRAM: Extract DEFAULT_MASK into variable

* BadRAM: Add is_covered() for checking if pattern is already covered by one of the existing patterns

* BadRAM: Initialize patterns to 0

* BadRAM: Change how addr/masks are merged to minimize number of addresses covered by badram

Prior to this patch, a list of up to MAX_PATTERNS (=10) addr/mask tuples
(aka. pattern) were maintained, adding failing addresses one by one to
the list until it was full. When full, space was created by forcing a
merge of the new address with the existing pattern that would grow the
least (with regards to number of addresses covered by the pattern) by
merging it with the new address. This can lead to a great imbalance in
the number of addresses covered by the patterns. Consider the following:

MAX_PATTERNS=4 (for illustrative purposes).
The following addresses are faulted and added to patterns:
0x00, 0x10, 0x20, 0x68, 0xa0, 0xb0, 0xc0, 0xd0

This is the end result with the implementation prior to this commit:

patterns = [
  (0x00, 0xe8),
  (0x00, 0x18),
  (0x68, 0xf8),
  (0x90, 0x98)
]
Total addresses covered: 120.

This commit changes how the merges are done, not only considering a
merge between the new address and existing patterns, but also between
existing patterns. It keeps the patterns in ascending order (by .addr)
in patterns, and a new address is always inserted into patterns (even if
num_patterns == MAX_PATTERNS, patterns is of MAX_PATTERNS+1 size). Then,
if num_patterns > MAX_PATTERNS, we find the pair of patterns (only
considering neighbours, assuming for any pattern i, i-1 or i+1 will
be the best candidate for a merge) that would be the cheapest to
merge (using the same metric as prior to this patch), and merge those.

With this commit, this is the result of the exact same sequence of
addresses as above:
[
  (0x00, 0xe0),
  (0x68, 0xf8),
  (0xa0, 0xe8),
  (0xc0, 0xe8)
]
Total addresses covered: 72.

A drawback of the current implementation (as compared to the prior)
is that it does not make any attempt at merging patterns until
num_patterns == MAX_PATTERNS, which can lead to having several patterns
that could've been merged into one at no additional cost. I.e.:

patterns = [
  (0x00, 0xf8),
  (0x08, 0xf8)
]
can appear, even if
patterns = [
  (0x00, 0xf0)
]
represents the exact same addresses with one pattern instead of two.

* fixup! BadRAM: Change how addr/masks are merged to minimize number of addresses covered by badram

Co-authored-by: Anders Wenhaug <anders.wenhaug@solutionseeker.no>
This commit is contained in:
Anders Wenhaug 2023-01-03 23:12:47 +01:00 committed by GitHub
parent 1fca6dbcab
commit 68deff493f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 123 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

177
app/badram.c
View File

@ -34,6 +34,7 @@
//------------------------------------------------------------------------------
#define MAX_PATTERNS 10
#define PATTERNS_SIZE (MAX_PATTERNS + 1)
// DEFAULT_MASK covers a uintptr_t, since that is the testing granularity.
#ifdef __x86_64__
@ -55,14 +56,16 @@ typedef struct {
// Private Variables
//------------------------------------------------------------------------------
static pattern_t pattern[MAX_PATTERNS];
static pattern_t patterns[PATTERNS_SIZE];
static int num_patterns = 0;
//------------------------------------------------------------------------------
// Private Functions
//------------------------------------------------------------------------------
#define COMBINE_MASK(a,b,c,d) ((a & b & c & d) | (~a & b & ~c & d))
// New mask is 1 where both masks were 1 (b & d) and the addresses were equal ~(a ^ c).
// If addresses were unequal the new mask must be 0 to allow for both values.
#define COMBINE_MASK(a,b,c,d) ((b & d) & ~(a ^ c))
/*
* Combine two addr/mask pairs to one addr/mask pair.
@ -72,7 +75,7 @@ static void combine(uintptr_t addr1, uintptr_t mask1, uintptr_t addr2, uintptr_t
*mask = COMBINE_MASK(addr1, mask1, addr2, mask2);
*addr = addr1 | addr2;
*addr &= *mask; // Normalise, no fundamental need for this
*addr &= *mask; // Normalise to ensure sorting on .addr will work as intended
}
/*
@ -104,58 +107,110 @@ static uintptr_t combi_cost(uintptr_t addr1, uintptr_t mask1, uintptr_t addr2, u
}
/*
* Find the cheapest array index to extend with the given addr/mask pair.
* Return -1 if nothing below the given minimum cost can be found.
* Determine if pattern is already covered by an existing pattern.
* Return true if that's the case, else false.
*/
static int cheap_index(uintptr_t addr1, uintptr_t mask1, uintptr_t min_cost)
static bool is_covered(pattern_t pattern)
{
int i = num_patterns;
int idx = -1;
while (i-- > 0) {
uintptr_t tmp_cost = combi_cost(pattern[i].addr, pattern[i].mask, addr1, mask1);
if (tmp_cost < min_cost) {
for (int i = 0; i < num_patterns; i++) {
if (combi_cost(patterns[i].addr, patterns[i].mask, pattern.addr, pattern.mask) == 0) {
return true;
}
}
return false;
}
/*
* Find the pair of entries that would be the cheapest to merge.
* Assumes patterns is sorted by .addr asc and that for each index i, the cheapest entry to merge with is at i-1 or i+1.
* Return -1 if <= 1 patterns exist, else the index of the first entry of the pair (the other being that + 1).
*/
static int cheapest_pair()
{
// This is guaranteed to be overwritten with >= 0 as long as num_patterns > 1
int merge_idx = -1;
uintptr_t min_cost = UINTPTR_MAX;
for (int i = 0; i < num_patterns - 1; i++) {
uintptr_t tmp_cost = combi_cost(
patterns[i].addr,
patterns[i].mask,
patterns[i+1].addr,
patterns[i+1].mask
);
if (tmp_cost <= min_cost) {
min_cost = tmp_cost;
idx = i;
merge_idx = i;
}
}
return idx;
return merge_idx;
}
/*
* Try to find a relocation index for idx if it costs nothing.
* Return -1 if no such index exists.
* Remove entries at idx and idx+1.
*/
static int relocate_index(int idx)
static void remove_pair(int idx)
{
uintptr_t addr = pattern[idx].addr;
uintptr_t mask = pattern[idx].mask;
pattern[idx].addr = ~pattern[idx].addr; // Never select idx
int new = cheap_index(addr, mask, 1 + addresses(mask));
pattern[idx].addr = addr;
return new;
for (int i = idx; i < num_patterns - 2; i++) {
patterns[i] = patterns[i + 2];
}
patterns[num_patterns - 1].addr = 0u;
patterns[num_patterns - 1].mask = 0u;
patterns[num_patterns - 2].addr = 0u;
patterns[num_patterns - 2].mask = 0u;
num_patterns -= 2;
}
/*
* Relocate the given index idx only if free of charge.
* This is useful to combine to `neighbouring' sections to integrate.
* Inspired on the Buddy memalloc principle in the Linux kernel.
* Get the combined entry of idx1 and idx2.
*/
static void relocate_if_free(int idx)
static pattern_t combined_pattern(int idx1, int idx2)
{
int newidx = relocate_index(idx);
if (newidx >= 0) {
uintptr_t caddr, cmask;
combine(pattern[newidx].addr, pattern[newidx].mask,
pattern[ idx].addr, pattern[ idx].mask,
&caddr, &cmask);
pattern[newidx].addr = caddr;
pattern[newidx].mask = cmask;
if (idx < --num_patterns) {
pattern[idx].addr = pattern[num_patterns].addr;
pattern[idx].mask = pattern[num_patterns].mask;
}
relocate_if_free (newidx);
pattern_t combined;
combine(
patterns[idx1].addr,
patterns[idx1].mask,
patterns[idx2].addr,
patterns[idx2].mask,
&combined.addr,
&combined.mask
);
return combined;
}
/*
* Insert pattern at index idx, shuffling other entries on index towards the end.
*/
static void insert_at(pattern_t pattern, int idx)
{
// Move all entries >= idx one index towards the end to make space for the new entry
for (int i = num_patterns - 1; i >= idx; i--) {
patterns[i + 1] = patterns[i];
}
patterns[idx] = pattern;
num_patterns++;
}
/*
* Insert entry (addr, mask) in patterns in an index i so that patterns[i-1].addr < patterns[i]
* NOTE: Assumes patterns is already sorted by .addr asc!
*/
static void insert_sorted(pattern_t pattern)
{
// Normalise to ensure sorting on .addr will work as intended
pattern.addr &= pattern.mask;
// Find index to insert entry into
int new_idx = num_patterns;
for (int i = 0; i < num_patterns; i++) {
if (pattern.addr < patterns[i].addr) {
new_idx = i;
break;
}
}
insert_at(pattern, new_idx);
}
//------------------------------------------------------------------------------
@ -165,26 +220,40 @@ static void relocate_if_free(int idx)
void badram_init(void)
{
num_patterns = 0;
for (int idx = 0; idx < PATTERNS_SIZE; idx++) {
patterns[idx].addr = 0u;
patterns[idx].mask = 0u;
}
}
bool badram_insert(uintptr_t addr)
{
if (cheap_index(addr, DEFAULT_MASK, 1) != -1) {
pattern_t pattern = {
.addr = addr,
.mask = DEFAULT_MASK
};
// If covered by existing entry we return immediately
if (is_covered(pattern)) {
return false;
}
if (num_patterns < MAX_PATTERNS) {
pattern[num_patterns].addr = addr;
pattern[num_patterns].mask = DEFAULT_MASK;
num_patterns++;
relocate_if_free(num_patterns - 1);
} else {
int idx = cheap_index(addr, DEFAULT_MASK, UINTPTR_MAX);
uintptr_t caddr, cmask;
combine(pattern[idx].addr, pattern[idx].mask, addr, DEFAULT_MASK, &caddr, &cmask);
pattern[idx].addr = caddr;
pattern[idx].mask = cmask;
relocate_if_free(idx);
// Add entry in order sorted by .addr asc
insert_sorted(pattern);
// If we have more patterns than the max we need to force a merge
if (num_patterns > MAX_PATTERNS) {
// Find the pair that is the cheapest to merge
// merge_idx will be -1 if num_patterns < 2, but that means MAX_PATTERNS = 0 which is not a valid state anyway
int merge_idx = cheapest_pair();
pattern_t combined = combined_pattern(merge_idx, merge_idx + 1);
// Remove the source pair so that we can maintain order as combined does not necessarily belong in merge_idx
remove_pair(merge_idx);
insert_sorted(combined);
}
return true;
}
@ -214,8 +283,8 @@ void badram_display(void)
col = 7;
}
display_scrolled_message(col, "0x%0*x,0x%0*x",
TESTWORD_DIGITS, pattern[i].addr,
TESTWORD_DIGITS, pattern[i].mask);
TESTWORD_DIGITS, patterns[i].addr,
TESTWORD_DIGITS, patterns[i].mask);
col += text_width;
}
}