-
Notifications
You must be signed in to change notification settings - Fork 83
Repartition data after loading IO class configuration #881
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
base: master
Are you sure you want to change the base?
Conversation
739b071 to
76c3d31
Compare
|
|
||
| part_id = ocf_metadata_get_partition_id(cache, cline); | ||
| part = &cache->user_parts[part_id].part; | ||
| part = &cache->user_parts[node->partition_id].part; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this change belongs to the previous commit :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed :)
| list = ocf_lru_get_list(part, curr_lru, iter->clean); | ||
|
|
||
| cline = list->tail; | ||
| while (cline != end_marker && !ocf_cache_line_try_lock_wr( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functions won't repart all cache lines because of this try_lock(). It will only repart the cache lines that is was able to lock. Any CL that would be locked by a concurrent IO request won't be reparted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed that if there is only the default IO class active then those cache lines which can't be locked would be reparted as a result of this concurrent IO.
| if (metadata_test_dirty(cache, cline)) | ||
| ocf_cleaning_purge_cache_block(cache, cline); | ||
|
|
||
| ocf_lru_repart_locked(cache, cline, part, dst_part); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think ocf_lru_repart_locked() is a good fit for repartition during a managment operation because every reparted cache line will be marked as hot and will trigger unnecessary rebalancing on the default ioclass.
src/mngt/ocf_mngt_io_class.c
Outdated
| _ocf_mngt_set_partition_size(cache, i, 0, 0); | ||
|
|
||
| for (i = 1; i < OCF_USER_IO_CLASS_MAX; i++) { | ||
| ocf_lru_repart_all(cache, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to repart all ioclasses? If the IO class configuration has changed shouldn't we repart only the partitions that has been disabled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 3rd commit in this PR adds an optional parameter that decides whether only non-active or all partitions should undergo the process.
Signed-off-by: Daniel Madej <[email protected]>
Signed-off-by: Daniel Madej <[email protected]>
With `all` parameter set repartition data from all IO classes. If `all` is unset repartition only from deleted IO classes. Signed-off-by: Daniel Madej <[email protected]>
No description provided.