Backfill optimization #834
Conversation
mmichal10
force-pushed
the
backfill-optimization
branch
2 times, most recently
from
53c508a to
1972ee1
Compare
A new status for cache lines that are partial hits Signed-off-by: Sara Merzel <[email protected]> Signed-off-by: Robert Baldyga <[email protected]> Signed-off-by: Michal Mielewczyk <[email protected]>
Signed-off-by: Michal Mielewczyk <[email protected]>
Simplify calculating addr and size. Getting rid of a few ifs shouldn't impair the performance as well! Signed-off-by: Michal Mielewczyk <[email protected]>
mmichal10
force-pushed
the
backfill-optimization
branch
from
1972ee1 to
b1c57af
Compare
There was a problem hiding this comment.
Seems OK. My only consideration is, if it wouldn't be better to use the existing implementation of ocf_engine_forward_cache_io_req() and just add the conditional if (req->map[i].status == LOOKUP_HIT) to it (as a flag maybe), instead of copying over the whole function content.
|
@rafalste I think the implementation is too specific for backfill to apply it in a generic function like |
| #define __is_the_last_chunk(__req, __i) (__i == (__req->core_line_count - 1)) | ||
|
|
||
| #define __skip_on_the_last_entry(__skip, __req, __i) \ | ||
| __skip * (int)__is_the_last_chunk(__req, __i) |
There was a problem hiding this comment.
It's no longer 90's, unlikely() should do.
| last_chunk_size = (req->addr + req->bytes) % cache_line_size; | ||
| skip = (cache_line_size - last_chunk_size) % cache_line_size; |
There was a problem hiding this comment.
This math is very confusing. The last_chunk_size is not really last chunk size, because it can be 0 if last chunk is aligned to cache line size, and then to account for that, the skip is "corrected" with this surprising modulo.
| total_bytes -= seek; | ||
| /* Seek should be taken into account for the first chunk | ||
| only */ | ||
| seek = 0; |
There was a problem hiding this comment.
I'd prefer unlikely() on if (i == 0). Now we are zeroing this seek here and there many times for completely no reason.
| addr += seek; | ||
| bytes -= seek; |
There was a problem hiding this comment.
And here we completely unnecessarily add/subtract 0 on every loop iteration.
No description provided.