From 3d6e30119b4b94cf592def12b8d29f73c1e4a6c0 Mon Sep 17 00:00:00 2001 From: Themis Demetriades Date: Fri, 6 Dec 2024 15:31:27 +0000 Subject: [PATCH] refactor: rename lru_lock to ftable_lock for greater clarity, and update comments to reflect this --- src/vm/frame.c | 29 ++++++++++++++--------------- src/vm/frame.h | 5 +++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/vm/frame.c b/src/vm/frame.c index c1d5ae3..1a6c9a1 100644 --- a/src/vm/frame.c +++ b/src/vm/frame.c @@ -40,8 +40,8 @@ struct frame_metadata within 'frame_table', whose key is the kernel virtual address of the frame. */ struct list_elem list_elem; /* Tracks the position of the frame metadata - in either the 'active' or 'inactive' list, - so a victim can be chosen for eviction. */ + within 'lru_list', so a victim can be + chosen for eviction. */ }; hash_hash_func frame_metadata_hash; @@ -63,7 +63,7 @@ frame_init (void) hash_init (&frame_table, frame_metadata_hash, frame_metadata_less, NULL); list_init (&lru_list); - lock_init (&lru_lock); + lock_init (&ftable_lock); } /* TODO: Consider synchronisation more closely (i.e. just for hash @@ -79,7 +79,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner) struct frame_metadata *frame_metadata; flags |= PAL_USER; - lock_acquire (&lru_lock); + lock_acquire (&ftable_lock); void *frame = palloc_get_page (flags); /* If a frame couldn't be allocated we must be out of main memory. Thus, @@ -122,7 +122,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner) memset (victim->frame, 0, PGSIZE); /* 3. Indicate that the new frame's metadata will be stored - inside the same structure that stored the victim's metadata. + inside the same structure that stored the victim's metadata.frame.c As both the new frame and the victim frame share the same kernel virtual address, the hash map need not be updated, and neither the list_elem value as both share the same lru_list position. */ @@ -133,7 +133,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner) we must update the frame table with a new entry, and grow lru_list. */ else { - /* Must own lru_lock here, as otherwise there is a race condition + /* Must own ftable_lock here, as otherwise there is a race condition with next_victim either being NULL or uninitialized. */ frame_metadata = malloc (sizeof (struct frame_metadata)); if (frame_metadata == NULL) @@ -165,7 +165,7 @@ frame_alloc (enum palloc_flags flags, void *upage, struct thread *owner) list_push_back (&frame_metadata->owners, &frame_owner->elem); frame_metadata->upage = upage; frame_metadata->pinned = false; - lock_release (&lru_lock); + lock_release (&ftable_lock); return frame_metadata->frame; } @@ -173,28 +173,28 @@ void frame_pin (void *frame) { ASSERT (frame != NULL); - lock_acquire (&lru_lock); + lock_acquire (&ftable_lock); struct frame_metadata *frame_metadata = frame_metadata_get (frame); if (frame_metadata == NULL) PANIC ("Attempted to pin a frame at an unallocated kernel address '%p'\n", frame); frame_metadata->pinned = true; - lock_release (&lru_lock); + lock_release (&ftable_lock); } void frame_unpin (void *frame) { ASSERT (frame != NULL); - lock_acquire (&lru_lock); + lock_acquire (&ftable_lock); struct frame_metadata *frame_metadata = frame_metadata_get (frame); if (frame_metadata == NULL) PANIC ("Attempted to unpin a frame at an unallocated kernel address '%p'\n", frame); frame_metadata->pinned = false; - lock_release (&lru_lock); + lock_release (&ftable_lock); } /* Attempt to deallocate a frame for a user process by removing it from the @@ -210,7 +210,7 @@ frame_free (void *frame) frame); free_owners (&frame_metadata->owners); - lock_acquire (&lru_lock); + lock_acquire (&ftable_lock); hash_delete (&frame_table, &frame_metadata->hash_elem); list_remove (&frame_metadata->list_elem); @@ -224,7 +224,7 @@ frame_free (void *frame) else next_victim = lru_next (next_victim); } - lock_release (&lru_lock); + lock_release (&ftable_lock); free (frame_metadata); palloc_free_page (frame); @@ -287,9 +287,8 @@ frame_metadata_find (void *frame) return hash_entry (e, struct frame_metadata, hash_elem); } -/* TODO: Account for page aliases when checking accessed bit. */ /* A pre-condition for calling this function is that the calling thread - owns lru_lock and that lru_list is non-empty. */ + owns ftable_lock and that lru_list is non-empty. */ static struct frame_metadata * get_victim (void) { diff --git a/src/vm/frame.h b/src/vm/frame.h index cb5916e..62a1ec6 100644 --- a/src/vm/frame.h +++ b/src/vm/frame.h @@ -11,8 +11,9 @@ struct frame_owner }; /* Synchronisation variables. */ -/* Protects access to 'lru_list'. */ -struct lock lru_lock; +/* Protects access to the frame table and its related components. */ +struct lock ftable_lock; + void frame_init (void); void *frame_alloc (enum palloc_flags, void *, struct thread *);