Skip to content

Optimize page fault handling by caching last accessed range#36

Open
jvbronzeado wants to merge 3 commits into
Mathewnd:rewritefrom
jvbronzeado:rewrite
Open

Optimize page fault handling by caching last accessed range#36
jvbronzeado wants to merge 3 commits into
Mathewnd:rewritefrom
jvbronzeado:rewrite

Conversation

@jvbronzeado

Copy link
Copy Markdown

No description provided.

Comment thread kernel-src/mm/mm.c Outdated

// we look at the last page fault range and check if the address is in this range
// this avoids more expensive tree lookup
mm_range_t* range = space->last_pagefault;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style nit: * goes in the right side

Comment thread kernel-src/mm/mm.c Outdated
// we look at the last page fault range and check if the address is in this range
// this avoids more expensive tree lookup
mm_range_t* range = space->last_pagefault;
if(range == NULL || range->start > addr || addr >= range->start + range->size) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style nit: space between if and (

Comment thread kernel-src/mm/range.c
@@ -1 +1 @@
#include <kernel/mm.h>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing handling for some cases like mm_insert_range coalescing. Make sure to check all places where mm_free_range() is called.

Comment thread kernel-src/mm/mm.c Outdated
mm_range_t *range = mm_get_range(space, addr);

// we look at the last page fault range and check if the address is in this range
// this avoids more expensive tree lookup

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "this might avoid a more expensive tree lookup"

Comment thread kernel-src/mm/range.c Outdated

if (free) {
// release page data
if (space->last_pagefault == range)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is only really needed in the complete unmapping case:

if (free) {
    // <----- insert it here
    rbtree_remove(&space->ranges, &range->rbtree_node);
    mm_destroy_range(range, 0, range->size, 0);
    mm_free_range(range);
}

in the other cases, the range is not really free'd and so it just checks against the range's new base address/size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants