Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate the inner structure of mpool. #80

Draft
wants to merge 1 commit into
base: upstream
Choose a base branch
from

Conversation

efenniht
Copy link
Collaborator

Note:

  • 바뀐 코드에서는 mpool_inner.entry_sizempool.fallback 이 락으로 보호되지 않습니다. 기존의 코드에서는 mpool_inner.entry_size 는 (필요 없이) 보호되었고 mpool.fallback 은 보호하는 코드가 있고 그렇지 않은 코드도 있었습니다.

TODO:

  • mpool_inner_appends_to 이름이 이상하고,
  • 이 함수 내에서 락을 사실 안 걸어도 되는데, C에서는 full ownership 개념이 없어서 그렇게 하는 게 올바른 리팩토링인지 잘 모르겠습니다.

@efenniht efenniht self-assigned this Mar 15, 2020
Copy link
Member

@jeehoonkang jeehoonkang left a comment

Choose a reason for hiding this comment

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

  • 이 PR은 실수로 머지하지 않도록 DRAFT로 만들어주시길 부탁드려요.
  • 우리끼리 리뷰를 마치면 hafnium upstream에 제안합시다.

struct spinlock lock;
size_t entry_size;
struct mpool_chunk *chunk_list;
struct mpool_entry *entry_list;
};

struct mpool {
Copy link
Member

Choose a reason for hiding this comment

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

  • mpool이 spin_lock, mpool_inner, fallback을 가지고 있는게 어떤가 합니다.
  • mpool_inner는 chunk_list, entry_list를 가지고 있구요.
  • entry_size는 없애고 4096? 으로 고정합시다. 어디 상수로 박아두고..
  • mpool_inner에 작용하는 함수는 lock 없이 내부를 접근하는 것으로 하고, mpool에서 inner에 접근할 때 spinlock을 잡도록 합시다.
  • invariant: mpool의 lock이 inner를 protect한다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 spinlock을 mpool_inner 에서 mpool로 옮겼습니다. entry_size는 삭제하려고 했는데, hafnium 에서는 무조건 4K지만 test code에서 다른 크기로 만들어서 사용하고 있습니다.

  • 테스트 코드(src/mpool_test.cc) 또한 다 수정하거나
  • entry_size는 그대로 두거나 해야 할 것 같습니다.

Copy link
Member

Choose a reason for hiding this comment

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

  • 저는 1번을 택했었는데... 2번의 경우 아래와 같은 어려운 점이 있습니다.
  • 지금 코드를 보면 lock을 안잡고 inner의 entry_size를 접근하는데, 말인즉슨 entry_size는 lock으로 protect되는 데이타가 아닙니다. 따라서 지금 구현은 lock이 inner를 protect한다는 전제와 충돌합니다. 이를 피하려면 entry_size를 mpool의 멤버로 두어야 합니다. 그런데 그러면 inner의 모든 operation에 entry_size를 인자로 넣어줘야 하고 코드가 지저분해집니다. 이를 피하려면 entry_size를 상수로 고정하는게 제일 간단할 것 같습니다.

@efenniht efenniht marked this pull request as draft April 15, 2020 06:23
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