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

ACP Cleaning Policy for Chunk with small dirty cache lines #286

Closed
MinneyJohn opened this issue Sep 25, 2019 · 4 comments
Closed

ACP Cleaning Policy for Chunk with small dirty cache lines #286

MinneyJohn opened this issue Sep 25, 2019 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@MinneyJohn
Copy link
Contributor

For ACP cleaning policy, there are two parameters which are "-w" and "-b", it means:

  • For each cycle, CAS will try to flush max "-b" cache blocks.
[-w, --wake-up <NUM>]: Period of time between awakenings of flushing thread in milliseconds. MIN: 0, MAX: 10000
(inclusive), DEFAULT: 10.
[-b, --flush-max-buffers <NUM>]: Number of dirty cache blocks to be flushed in one cleaning cycle. MIN: 1, MAX:
10000 (inclusive), DEFAULT: 128.

But seems there is another factor which may determine the flush speed of ACP:

  • The dirty cache blocks in the chunk
    Here is how the ACP works for every cycle:
  • First, choose one chunk (with most percentage of dirty blocks)
  • Then try to find max "-b" dirty cache blocks from the chunk and issue flush IO to core device
    • If the dirty cache blocks in the chunk is small (eg. smaller than "-b"), only a small number of dirty cache blocks will be flushed to core device
  • Wait for "-w" and repeat the process
void cleaning_policy_acp_perform_cleaning(ocf_cache_t cache,
		ocf_cleaner_end_t cmpl)
{
	struct acp_cleaning_policy_config *config;
	struct acp_context *acp = _acp_get_ctx_from_cache(cache);
	struct acp_state *state = &acp->state;

	acp->cmpl = cmpl;

	if (!state->in_progress) {
		/* get next chunk to clean */
		state->chunk = _acp_get_cleaning_candidate(cache);

		if (!state->chunk) {
			/* nothing co clean */
			cmpl(&cache->cleaner, ACP_BACKOFF_TIME_MS);
			return;
		}

		/* new cleaning cycle - reset state */
		state->iter = 0;
		state->in_progress = true;
	}

	ACP_DEBUG_INIT(acp);

	config = (void *)&cache->conf_meta->cleaning[ocf_cleaning_acp].data;

	if (_acp_prepare_flush_data(acp, config->flush_max_buffers))
		_acp_flush(acp);
	else
		_acp_flush_end(acp, 0);
}

Or in other words, with our current coding, here is how ACP works:

  • For each "-w" cycle, the number of dirty cache blocks flushed to core device is the min of "-b" and the dirty cache blocks in the chunks.
@MinneyJohn
Copy link
Contributor Author

Not sure whether we need to update function cleaning_policy_acp_perform_cleaning to make it real flush "-b" dirty cache blocks no matter how much dirty blocks are in one chunk.

@jfckm
Copy link

jfckm commented Oct 14, 2019

I think that the real error here is just wording in CLI. Maybe it should be something like **Maximum** number of dirty cache blocks to be flushed in one cleaning cycle?

As for ACP policy flushing -b dirty blocks per iteration I that that was not the behavior we were going for. Any comments on that @arutk ?

@arutk
Copy link
Contributor

arutk commented Oct 14, 2019

I believe the original intention was to have ACP clean exactly "-b" dirty cache lines. I agree with John's analysis - ACP will issue smaller I/O at the end of chunk.

So it looks like implementation shortcoming. Possible fix would involve:

  1. modifying acp_perform_cleaning() to issue second flush I/O if the first call had not reached the requested flush package size.
    We probably shouldn't iterate over more than two chunks in a single iteration, since this means that the cache is basically clean and cleaner performance is not relevant at this point.
  2. _acp_prepare_flush_data() and _acp_flush() would accept offset parameter to indicate start position in in state->flush array.
  3. Modify _acp_flush_end() to wait for the second I/O before calling upper layer completion callback.

@jfckm jfckm added the enhancement New feature or request label Jul 16, 2020
@arutk
Copy link
Contributor

arutk commented Feb 3, 2022

This doesn't seem to be a noticeable problem, as ACP by design cleans the chunks that are most dirty. Assuming there is some workload LBA locality (which is a reasonable assumption for cache), the core LBA range (chunk) being cleaned has relatively large number of dirty cachelines. So one would need to set very large flush portion to make this problem noticable (i.e. measure reduced cleaner bandwidth).

We haven't heard from any user hitting this problem in real life scenarios, so closing this .

@arutk arutk closed this as completed Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants