From a1af1809d8fb146ac22293f724ecdef95303f8b8 Mon Sep 17 00:00:00 2001 From: Robert Baldyga Date: Mon, 14 Oct 2024 16:54:51 +0200 Subject: [PATCH] Fix error accounting in forward_io Resetting cache_error/core_error in ocf_req_forward_* functions may lead to overwriting already reported error if the forward is being done in the loop. To avoid this potential problem, introduce set of forward init functions intended to be called before the entire forward operation, which resets the error code and sets a forward callback. Signed-off-by: Robert Baldyga --- src/engine/engine_io.c | 14 +++++----- src/engine/engine_wo.c | 2 +- src/metadata/metadata_io.c | 4 +-- src/metadata/metadata_raw_atomic.c | 2 +- src/metadata/metadata_raw_dynamic.c | 2 +- src/metadata/metadata_superblock.c | 2 +- src/ocf_cache.c | 6 ++--- src/ocf_request.c | 42 ++++++++++++++++------------- src/ocf_request.h | 14 +++++++--- src/ocf_volume.c | 8 +++--- src/utils/utils_cleaner.c | 9 +++---- src/utils/utils_io.c | 6 ++--- 12 files changed, 61 insertions(+), 50 deletions(-) diff --git a/src/engine/engine_io.c b/src/engine/engine_io.c index cb7f79c1..73433bcb 100644 --- a/src/engine/engine_io.c +++ b/src/engine/engine_io.c @@ -20,7 +20,7 @@ void ocf_engine_forward_cache_io(struct ocf_request *req, int dir, uint32_t first_cl = ocf_bytes_2_lines(cache, offset + seek); uint64_t addr; - req->cache_forward_end = callback; + ocf_req_forward_cache_init(req, callback); addr = cache->device->metadata_offset; addr += req->map[first_cl].coll_idx * ocf_line_size(cache); @@ -40,7 +40,7 @@ void ocf_engine_forward_cache_io_req(struct ocf_request *req, int dir, uint64_t addr, bytes, total_bytes = 0, addr_next = 0; uint32_t i; - req->cache_forward_end = callback; + ocf_req_forward_cache_init(req, callback); if (ocf_engine_is_sequential(req)) { addr = cache->device->metadata_offset; @@ -113,7 +113,7 @@ void ocf_engine_forward_cache_io_req(struct ocf_request *req, int dir, void ocf_engine_forward_cache_flush_req(struct ocf_request *req, ocf_req_end_t callback) { - req->cache_forward_end = callback; + ocf_req_forward_cache_init(req, callback); ocf_req_forward_cache_flush(req); } @@ -121,7 +121,7 @@ void ocf_engine_forward_cache_flush_req(struct ocf_request *req, void ocf_engine_forward_cache_discard_req(struct ocf_request *req, ocf_req_end_t callback) { - req->cache_forward_end = callback; + ocf_req_forward_cache_init(req, callback); ocf_req_forward_cache_discard(req, req->addr, req->bytes); @@ -133,7 +133,7 @@ void ocf_engine_forward_core_io_req(struct ocf_request *req, ocf_core_stats_core_block_update(req->core, req->part_id, req->rw, req->bytes); - req->core_forward_end = callback; + ocf_req_forward_core_init(req, callback); ocf_req_forward_core_io(req, req->rw, req->addr, req->bytes, req->offset); @@ -145,7 +145,7 @@ void ocf_engine_forward_core_flush_req(struct ocf_request *req, ocf_core_stats_core_block_update(req->core, req->part_id, req->rw, req->bytes); - req->core_forward_end = callback; + ocf_req_forward_core_init(req, callback); ocf_req_forward_core_flush(req); } @@ -156,7 +156,7 @@ void ocf_engine_forward_core_discard_req(struct ocf_request *req, ocf_core_stats_core_block_update(req->core, req->part_id, req->rw, req->bytes); - req->core_forward_end = callback; + ocf_req_forward_core_init(req, callback); ocf_req_forward_core_discard(req, req->addr, req->bytes); } diff --git a/src/engine/engine_wo.c b/src/engine/engine_wo.c index 11bce5bf..16a93625 100644 --- a/src/engine/engine_wo.c +++ b/src/engine/engine_wo.c @@ -62,7 +62,7 @@ static int ocf_read_wo_cache_do(struct ocf_request *req) uint64_t offset = 0; uint64_t increment = 0; - req->cache_forward_end = ocf_read_wo_cache_complete; + ocf_req_forward_cache_init(req, ocf_read_wo_cache_complete); ocf_req_forward_cache_get(req); diff --git a/src/metadata/metadata_io.c b/src/metadata/metadata_io.c index e48d7664..a18960a6 100644 --- a/src/metadata/metadata_io.c +++ b/src/metadata/metadata_io.c @@ -111,7 +111,7 @@ static int metadata_io_read_i_atomic_step(struct ocf_request *req) /* Reset position in data buffer */ ctx_data_seek(cache->owner, req->data, ctx_data_seek_begin, 0); - req->cache_forward_end = metadata_io_read_i_atomic_step_end; + ocf_req_forward_cache_init(req, metadata_io_read_i_atomic_step_end); ocf_req_forward_cache_metadata(req, OCF_READ, cache->device->metadata_offset + @@ -217,7 +217,7 @@ static int metadata_io_do(struct ocf_request *req) ctx_data_seek(cache->owner, req->data, ctx_data_seek_begin, 0); - req->cache_forward_end = metadata_io_end; + ocf_req_forward_cache_init(req, metadata_io_end); ocf_req_forward_cache_io(req, req->rw, PAGES_TO_BYTES(m_req->page), PAGES_TO_BYTES(m_req->count), 0); diff --git a/src/metadata/metadata_raw_atomic.c b/src/metadata/metadata_raw_atomic.c index 6d78a2b4..dd6b9b51 100644 --- a/src/metadata/metadata_raw_atomic.c +++ b/src/metadata/metadata_raw_atomic.c @@ -118,7 +118,7 @@ int raw_atomic_flush_do_asynch(struct ocf_cache *cache, struct ocf_request *req, } req->priv = complete; - req->cache_forward_end = _raw_atomic_io_discard_cmpl; + ocf_req_forward_cache_init(req, _raw_atomic_io_discard_cmpl); if (line_no == 1) { map = &req->map[0]; diff --git a/src/metadata/metadata_raw_dynamic.c b/src/metadata/metadata_raw_dynamic.c index 1a8a2b4f..481fc01d 100644 --- a/src/metadata/metadata_raw_dynamic.c +++ b/src/metadata/metadata_raw_dynamic.c @@ -402,7 +402,7 @@ static int raw_dynamic_load_all_read(struct ocf_request *req) count = metadata_io_size(context->i_page, raw->ssd_pages); - req->cache_forward_end = raw_dynamic_load_all_read_end; + ocf_req_forward_cache_init(req, raw_dynamic_load_all_read_end); ocf_req_forward_cache_io(req, OCF_READ, PAGES_TO_BYTES(ssd_pages_offset + context->i_page), diff --git a/src/metadata/metadata_superblock.c b/src/metadata/metadata_superblock.c index bdd25af0..f72761b6 100644 --- a/src/metadata/metadata_superblock.c +++ b/src/metadata/metadata_superblock.c @@ -705,7 +705,7 @@ int ocf_metadata_read_sb(ocf_ctx_t ctx, ocf_volume_t volume, goto err_data; } - req->volume_forward_end = ocf_metadata_read_sb_complete; + ocf_req_forward_volume_init(req, ocf_metadata_read_sb_complete); req->priv = context; ocf_req_forward_volume_io_simple(req, volume, OCF_READ, 0, diff --git a/src/ocf_cache.c b/src/ocf_cache.c index b3d0deeb..2610940d 100644 --- a/src/ocf_cache.c +++ b/src/ocf_cache.c @@ -332,7 +332,7 @@ static void ocf_cache_volume_submit_io(ocf_io_t io) env_atomic_set(&req->req_remaining, 3); - req->cache_forward_end = ocf_cache_io_complete; + ocf_req_forward_cache_init(req, ocf_cache_io_complete); ocf_req_forward_cache_io(req, req->rw, req->addr, req->bytes, req->offset); @@ -361,7 +361,7 @@ static void ocf_cache_volume_submit_flush(ocf_io_t io) return; } - req->cache_forward_end = ocf_cache_volume_io_complete_generic; + ocf_req_forward_cache_init(req, ocf_cache_volume_io_complete_generic); ocf_req_forward_cache_flush(req); } @@ -380,7 +380,7 @@ static void ocf_cache_volume_submit_discard(ocf_io_t io) return; } - req->cache_forward_end = ocf_cache_volume_io_complete_generic; + ocf_req_forward_cache_init(req, ocf_cache_volume_io_complete_generic); ocf_req_forward_cache_discard(req, req->addr, req->bytes); } diff --git a/src/ocf_request.c b/src/ocf_request.c index 5994debb..bc144331 100644 --- a/src/ocf_request.c +++ b/src/ocf_request.c @@ -440,6 +440,16 @@ void ocf_io_end_func(ocf_io_t io, int error) req->io.end(io, req->io.priv1, req->io.priv2, error); } +void ocf_req_forward_volume_init(struct ocf_request *req, + ocf_req_end_t callback) +{ + /* For forward_volume we reuse cache_* fields. This kind of forward + operation should never happen together with forward_cache. + */ + req->cache_error = 0; + req->cache_forward_end = callback; +} + void ocf_req_forward_volume_io(struct ocf_request *req, ocf_volume_t volume, int dir, uint64_t addr, uint64_t bytes, uint64_t offset) { @@ -471,20 +481,23 @@ void ocf_req_forward_volume_io_simple(struct ocf_request *req, { ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_io_simple(volume, token, dir, addr, bytes); } +void ocf_req_forward_cache_init(struct ocf_request *req, + ocf_req_end_t callback) +{ + req->cache_error = 0; + req->cache_forward_end = callback; +} + void ocf_req_forward_cache_io(struct ocf_request *req, int dir, uint64_t addr, uint64_t bytes, uint64_t offset) { ocf_volume_t volume = ocf_cache_get_volume(req->cache); ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_io(volume, token, dir, addr, bytes, offset); } @@ -494,8 +507,6 @@ void ocf_req_forward_cache_flush(struct ocf_request *req) ocf_volume_t volume = ocf_cache_get_volume(req->cache); ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_flush(volume, token); } @@ -506,8 +517,6 @@ void ocf_req_forward_cache_discard(struct ocf_request *req, uint64_t addr, ocf_volume_t volume = ocf_cache_get_volume(req->cache); ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_discard(volume, token, addr, bytes); } @@ -518,8 +527,6 @@ void ocf_req_forward_cache_write_zeros(struct ocf_request *req, uint64_t addr, ocf_volume_t volume = ocf_cache_get_volume(req->cache); ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_write_zeros(volume, token, addr, bytes); } @@ -530,20 +537,23 @@ void ocf_req_forward_cache_metadata(struct ocf_request *req, int dir, ocf_volume_t volume = ocf_cache_get_volume(req->cache); ocf_forward_token_t token = ocf_req_to_cache_forward_token(req); - req->cache_error = 0; - ocf_req_forward_cache_get(req); ocf_volume_forward_metadata(volume, token, dir, addr, bytes, offset); } +void ocf_req_forward_core_init(struct ocf_request *req, + ocf_req_end_t callback) +{ + req->core_error = 0; + req->core_forward_end = callback; +} + void ocf_req_forward_core_io(struct ocf_request *req, int dir, uint64_t addr, uint64_t bytes, uint64_t offset) { ocf_volume_t volume = ocf_core_get_volume(req->core); ocf_forward_token_t token = ocf_req_to_core_forward_token(req); - req->core_error = 0; - ocf_req_forward_core_get(req); ocf_volume_forward_io(volume, token, dir, addr, bytes, offset); } @@ -553,8 +563,6 @@ void ocf_req_forward_core_flush(struct ocf_request *req) ocf_volume_t volume = ocf_core_get_volume(req->core); ocf_forward_token_t token = ocf_req_to_core_forward_token(req); - req->core_error = 0; - ocf_req_forward_core_get(req); ocf_volume_forward_flush(volume, token); } @@ -565,8 +573,6 @@ void ocf_req_forward_core_discard(struct ocf_request *req, uint64_t addr, ocf_volume_t volume = ocf_core_get_volume(req->core); ocf_forward_token_t token = ocf_req_to_core_forward_token(req); - req->core_error = 0; - ocf_req_forward_core_get(req); ocf_volume_forward_discard(volume, token, addr, bytes); } diff --git a/src/ocf_request.h b/src/ocf_request.h index ce62ac76..36ab4d80 100644 --- a/src/ocf_request.h +++ b/src/ocf_request.h @@ -172,10 +172,7 @@ struct ocf_request { /* This struct is temporary. It will be consolidated with ocf_request */ struct ocf_request_io io; - union { - ocf_req_end_t cache_forward_end; - ocf_req_end_t volume_forward_end; - }; + ocf_req_end_t cache_forward_end; ocf_req_end_t core_forward_end; env_atomic cache_remaining; env_atomic core_remaining; @@ -602,6 +599,9 @@ static inline struct ocf_request *ocf_req_forward_token_to_req(ocf_forward_token return (struct ocf_request *)(token & ~1); } +void ocf_req_forward_volume_init(struct ocf_request *req, + ocf_req_end_t callback); + void ocf_req_forward_volume_io(struct ocf_request *req, ocf_volume_t volume, int dir, uint64_t addr, uint64_t bytes, uint64_t offset); @@ -613,6 +613,9 @@ void ocf_req_forward_volume_discard(struct ocf_request *req, void ocf_req_forward_volume_io_simple(struct ocf_request *req, ocf_volume_t volume, int dir, uint64_t addr, uint64_t bytes); +void ocf_req_forward_cache_init(struct ocf_request *req, + ocf_req_end_t callback); + void ocf_req_forward_cache_io(struct ocf_request *req, int dir, uint64_t addr, uint64_t bytes, uint64_t offset); @@ -627,6 +630,9 @@ void ocf_req_forward_cache_write_zeros(struct ocf_request *req, uint64_t addr, void ocf_req_forward_cache_metadata(struct ocf_request *req, int dir, uint64_t addr, uint64_t bytes, uint64_t offset); +void ocf_req_forward_core_init(struct ocf_request *req, + ocf_req_end_t callback); + void ocf_req_forward_core_io(struct ocf_request *req, int dir, uint64_t addr, uint64_t bytes, uint64_t offset); diff --git a/src/ocf_volume.c b/src/ocf_volume.c index 0b9ace47..3d8f6da1 100644 --- a/src/ocf_volume.c +++ b/src/ocf_volume.c @@ -281,7 +281,7 @@ ocf_io_t ocf_volume_new_io(ocf_volume_t volume, ocf_queue_t queue, return ocf_io_new(volume, queue, addr, bytes, dir, io_class, flags); } -static void ocf_volume_req_forward_complete(struct ocf_request *req, int error) +static void ocf_volume_req_forward_end(struct ocf_request *req, int error) { ocf_io_end_func(req, error); } @@ -299,7 +299,7 @@ void ocf_volume_submit_io(ocf_io_t io) if (likely(volume->type->properties->ops.submit_io)) { volume->type->properties->ops.submit_io(io); } else { - req->volume_forward_end = ocf_volume_req_forward_complete; + ocf_req_forward_volume_init(req, ocf_volume_req_forward_end); ocf_req_forward_volume_io(req, volume, req->rw, req->addr, req->bytes, req->offset); } @@ -318,7 +318,7 @@ void ocf_volume_submit_flush(ocf_io_t io) if (likely(volume->type->properties->ops.submit_flush)) { volume->type->properties->ops.submit_flush(io); } else { - req->volume_forward_end = ocf_volume_req_forward_complete; + ocf_req_forward_volume_init(req, ocf_volume_req_forward_end); ocf_req_forward_volume_flush(req, volume); } } @@ -336,7 +336,7 @@ void ocf_volume_submit_discard(ocf_io_t io) if (likely(volume->type->properties->ops.submit_discard)) { volume->type->properties->ops.submit_discard(io); } else { - req->volume_forward_end = ocf_volume_req_forward_complete; + ocf_req_forward_volume_init(req, ocf_volume_req_forward_end); ocf_req_forward_volume_discard(req, volume, req->addr, req->bytes); } diff --git a/src/utils/utils_cleaner.c b/src/utils/utils_cleaner.c index 7f2a8f36..3178efd8 100644 --- a/src/utils/utils_cleaner.c +++ b/src/utils/utils_cleaner.c @@ -290,7 +290,7 @@ static int _ocf_cleaner_fire_flush_cache(struct ocf_request *req) { OCF_DEBUG_TRACE(req->cache); - req->cache_forward_end = _ocf_cleaner_flush_cache_end; + ocf_req_forward_cache_init(req, _ocf_cleaner_flush_cache_end); ocf_req_forward_cache_flush(req); return 0; @@ -390,9 +390,8 @@ static void _ocf_cleaner_flush_core_end(struct ocf_request *req, int error) static int _ocf_cleaner_fire_flush_core(struct ocf_request *req) { - req->core_forward_end = _ocf_cleaner_flush_core_end; - /* Submit flush request */ + ocf_req_forward_core_init(req, _ocf_cleaner_flush_core_end); ocf_req_forward_core_flush(req); return 0; @@ -494,7 +493,7 @@ static int _ocf_cleaner_fire_core(struct ocf_request *req) OCF_DEBUG_TRACE(req->cache); - req->core_forward_end = _ocf_cleaner_core_io_end; + ocf_req_forward_core_init(req, _ocf_cleaner_core_io_end); /* Submits writes to the core */ ocf_req_forward_core_get(req); @@ -559,7 +558,7 @@ static int _ocf_cleaner_fire_cache(struct ocf_request *req) uint64_t addr, offset; ocf_part_id_t part_id; - req->cache_forward_end = _ocf_cleaner_cache_io_end; + ocf_req_forward_cache_init(req, _ocf_cleaner_cache_io_end); req->bytes = ocf_line_size(cache); ocf_req_forward_cache_get(req); diff --git a/src/utils/utils_io.c b/src/utils/utils_io.c index d180d121..d9358431 100644 --- a/src/utils/utils_io.c +++ b/src/utils/utils_io.c @@ -46,7 +46,7 @@ void ocf_submit_cache_flush(ocf_cache_t cache, return; } - req->cache_forward_end = ocf_submit_cache_end; + ocf_req_forward_cache_init(req, ocf_submit_cache_end); req->priv = context; ocf_req_forward_cache_flush(req); @@ -75,7 +75,7 @@ void ocf_submit_cache_discard(ocf_cache_t cache, uint64_t addr, return; } - req->cache_forward_end = ocf_submit_cache_end; + ocf_req_forward_cache_init(req, ocf_submit_cache_end); req->priv = context; ocf_req_forward_cache_get(req); @@ -181,7 +181,7 @@ void ocf_submit_cache_page(ocf_cache_t cache, uint64_t addr, int dir, req->data = data; - req->cache_forward_end = ocf_submit_cache_page_end; + ocf_req_forward_cache_init(req, ocf_submit_cache_page_end); req->priv = context; req->rw = dir; req->addr = addr;