정글 pintos-project3 table_kill시 hash_destroy하면 안되는 이유

배경

---

void supplemental_page_table_kill (struct supplemental_page_table *spt);

Frees all the resources that were held by a supplemental page table. This function is called when a process exits (process_exit() in userprog/process.c). You need to iterate through the page entries and call destroy(page) for the pages in the table. You do not need to worry about the actual page table (pml4) and the physical memory (palloc-ed memory) in this function; the caller cleans them after the supplemental page table is cleaned up.

• Anopnymous Page를 구현하는 부분에서 supplemental_page_table_kill로  생성했던 보조 테이블을 정리하는 부분이 나온다.
• 설명에 따르면 supplemental page table에 의해 잡혀있던 리소스들을 모두 free 해주라고 한다.
• 당연히 hash의 element 뿐만 아니라 hash까지 해제하는 hash_destroy 함수를 이용했다.

void
supplemental_page_table_kill (struct supplemental_page_table *spt) {
	/* TODO: Destroy all the supplemental_page_table hold by thread and
	 * TODO: writeback all the modified contents to the storage. */
	hash_destroy(&spt->pages,hash_page_destroy);
}

void hash_page_destroy(struct hash_elem *e, void *aux){
	const struct page *p = hash_entry (e, struct page, hash_elem);
	//swap, filebacked 처리
	if(page_get_type(p)==VM_FILE){
		swap_out(p);
	}
	vm_dealloc_page(p);
}

문제

• 그렇지만 이렇게 하면 아래와 같은 에러가 발생한다.

find_elem에서 page_fault가 발생한다.

• spt_find_page ->  hash_find-> find_elem 시 발생하는데 해당 부분을 디버그해보면

문제되는 부분

• hash의 bucket(리스트로 이루어짐) 에서 for문을 돌때, bucket의 head를 찾는데 이부분이 0x0. 즉 NULL이 나온다는 걸 알 수 있다.
• 정상적인 경우로 bucket 안에 요소가 아무것도 없다고 해도, list_begin은 list_end와 같은 값(즉 tail)을 가리키면서 for문이 실행되지 않아야 한다.
• 하지만 의미없는 값인 Null을 가리켜서 list_end와의 비교는 통과하고, 계속 null인 채로 hi도 만들고, 다음 if문의 less 함수를 실행하다가 General Protection Exception이 나는 것이다. (0x0 접근)

원인 분석

• 이렇게 문제가 발생하는 원인은 project2에서 흔히 하는 실수인 list_init()을 해주지 않았기 때문이다.
• 각 hash에 대한 list_init은  hash_init에서 해준다. (hash_init안에 있는 hash_clear에서 해준다.)

bool
hash_init (struct hash *h,
		hash_hash_func *hash, hash_less_func *less, void *aux) {
	
    //기타 초기화

	if (h->buckets != NULL) {
		hash_clear (h, NULL);   //hash_clear 호출
		return true;
	} else
		return false;
}

void
hash_clear (struct hash *h, hash_action_func *destructor) {
	size_t i;

	for (i = 0; i < h->bucket_cnt; i++) {
		struct list *bucket = &h->buckets[i];

		//생략

		list_init (bucket); //여기서 list init
	}

	h->elem_cnt = 0;
}

• 나는 hash_init을 잘 해줬는데 왜 init이 안 된 것처럼 작동할까?

원인 분석2

• hash_init 시에 hash값이 유의미하게 들어간 것은 잘 확인했다.
• 그렇다면 중간에 hash_init을 죽이는 부분이 있나?
  • 그렇다. process_exec을 실행할 때 시작하는 process_cleanup 함수 내부에 우리가 구현하려는 supplemental_page_table이 있다.
  • 이 내부에서 hash_destroy를 하면  hash_init이전으로 돌아가버리는 것이다.
• 깃북에 hash_destroy에 대해서 이렇게 적혀있다.

void hash_destroy (struct hash *hash, hash_action_func *action);

If action is non-null, calls it for each element in the hash, with the same semantics as a call to hash_clear(). Then, frees the memory held by hash. Afterward, hash must not be passed to any hash table function, absent an intervening call to hash_init().

• hash_clear를 하고 난 후 hash가 들고있던 메모리도 해제하기 때문에 destroy를 하고 난 후에는 다시 hash_init()을 하기 전까지는 다른 hash table을 써서는 안된다. (버그남)

 

• 실제로 process_cleanup() 전후로 bucket의 list_begin값이 바뀌는 걸 확인할 수 있다.
  • 디버그를 위해 코드를 추가로 넣음

int
process_exec (void *f_name) {
	char *file_name = f_name;
	bool success;
    
    //..생략

	//추가코드
	struct hash *hash = &(&thread_current()->spt)->pages;
	struct list_elem *i = list_begin (&hash->buckets[0]);
	
	/* We first kill the current context */
	process_cleanup ();   //cleanup 함수

	//..이하 생략

}

• process_cleanup 전 : prev가 잘 찍힌다.(list의 head 주소를 가리킴)

주소가 잘 찍히고 있는 편안한 모습

• process_cleanup() 이후 : list_begin값이 바뀜

작살나버린 list_begin

해결

• 두가지 방법이 있다.
• 첫번째는, 아래와 같이 hash_destroy가 아닌 hash까지는 안 지우고 요소들만 지우는 hash_clean을 쓰는 것이다.
  • 내가 쓰는 방법이다.

void
supplemental_page_table_kill (struct supplemental_page_table *spt) {
	/* TODO: Destroy all the supplemental_page_table hold by thread and
	 * TODO: writeback all the modified contents to the storage. */
	hash_clean(&spt->pages,hash_page_destroy);
}

• 두번째는 process_cleanup() 이후에 hash_init()을 한 번 더 해주는 것이다.
  • 동기들이 많이 참고한 github 코드 중 한 분이 사용한 방식이다.

해결 방법에 대한 고찰

• skeleton코드는 최대한 그대로 유지하자는 입장에서는 hash_clear를 사용하는 첫번째 방법이 더 좋은 방법이라고 생각한다.
• 이유는 다음과 같다.
  • hash_init을 두 번 하는 건 쓸 데 없이 반복코드이다.
  • 반복을 없애자고 앞부분의 supplemental_page_table_init()을 clean_up 이후로 옮기는 것도 skeleton코드를 파괴하는 행위이다.
• 단점도 있다.
  • hash_clear만 하고 끝나면 hash가 init한 bucket들의 메모리는 계속 남아있을 수 있다.
  • 따라서 process_exit의 process_cleanup() 이후에  hash_destroy()를 한 번 더 해줘야 한다.
• 단점이라고 썼지만, 구현 선택에 따른 추가코드일 뿐이라고 생각한다.
  • process_cleanup은 process의 context만 청소하는 것이기 때문에 context를 담는 컨테이너 자체를 파괴하면 안된다.
  • intr_frame을 초기화 하겠다고 intr_frame 속성 자체를 삭제하면 안되듯이

끝