From 07c7db81f4d53feb0a1a4aa0e98310b0f088f641 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Mon, 16 May 2022 13:32:36 +0200 Subject: [PATCH 1/6] pyocf: add FLUSH flag Flush I/O must be recognized by the bottom adapter by inspecting adapter specific flags Signed-off-by: Adam Rutkowski --- tests/functional/pyocf/types/volume.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/functional/pyocf/types/volume.py b/tests/functional/pyocf/types/volume.py index af90e58..fdee609 100644 --- a/tests/functional/pyocf/types/volume.py +++ b/tests/functional/pyocf/types/volume.py @@ -22,6 +22,7 @@ from ctypes import ( ) from hashlib import md5 import weakref +from enum import IntEnum from .io import Io, IoOps, IoDir from .queue import Queue @@ -32,6 +33,10 @@ from .data import Data from .queue import Queue +class IoFlags(IntEnum): + FLUSH = 1 + + class VolumeCaps(Structure): _fields_ = [("_atomic_writes", c_uint32, 1)] @@ -350,6 +355,11 @@ class RamVolume(Volume): discard.contents._end(discard, -OcfErrorCode.OCF_ERR_NOT_SUPP) def do_submit_io(self, io): + flags = int(io.contents._flags) + if flags & IoFlags.FLUSH: + self.do_submit_flush(io) + return + try: io_priv = cast(OcfLib.getInstance().ocf_io_get_priv(io), POINTER(VolumeIoPriv)) offset = io_priv.contents._offset From 8e16a26b6aa93c992b415fffe39af54f53c6c27c Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Mon, 16 May 2022 12:49:36 +0200 Subject: [PATCH 2/6] pyocf: add volume submit_flush() and submit_discard() Signed-off-by: Adam Rutkowski --- tests/functional/pyocf/types/io.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/functional/pyocf/types/io.py b/tests/functional/pyocf/types/io.py index bc26d52..ebd99f7 100644 --- a/tests/functional/pyocf/types/io.py +++ b/tests/functional/pyocf/types/io.py @@ -102,6 +102,12 @@ class Io(Structure): def submit_discard(self): return OcfLib.getInstance().ocf_volume_submit_discard(byref(self)) + def submit_flush(self): + return OcfLib.getInstance().ocf_volume_submit_flush(byref(self)) + + def submit_discard(self): + return OcfLib.getInstance().ocf_volume_submit_discard(byref(self)) + def set_data(self, data: Data, offset: int = 0): self.data = data OcfLib.getInstance().ocf_io_set_data(byref(self), data, offset) From df7ed6920cc73ed00ec27f265563154354e51dcf Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Mon, 16 May 2022 14:16:36 +0200 Subject: [PATCH 3/6] Fix ops(flush) engine Flush I/O should be forwarded to core and cache device. In case of core this is simple - just mirror the I/O from the top volume. Since cache data is owned by OCF it makes sense to send a simple flush I/O with 0 address and size. Current implementation attempts to use cache data I/O interface (ocf_submit_cache_reqs function) instead of submitting empty flush to the underlying cache device. This function is designed to read/write from mapped cachelines while there is no traversation/mapping performed on flush I/O. If request map allocation succeeds, this results in sending I/O to addres 0 with size and flags inherited from the top adapter I/O. This doesn't make any sense, and can even result in invalid I/O if the size is greater than cache device size. Even worse, if flush request map allocation fails (which happens always in case of large flush requests) then the erroneous call to ocf_submit_cache_reqs results in NULL pointer dereference. Signed-off-by: Adam Rutkowski --- src/engine/engine_ops.c | 11 ++++------- src/utils/utils_io.c | 17 +++++++++++++++++ src/utils/utils_io.h | 4 +++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/engine/engine_ops.c b/src/engine/engine_ops.c index a0b9beb..c65fc0b 100644 --- a/src/engine/engine_ops.c +++ b/src/engine/engine_ops.c @@ -1,5 +1,5 @@ /* - * Copyright(c) 2012-2021 Intel Corporation + * Copyright(c) 2012-2022 Intel Corporation * SPDX-License-Identifier: BSD-3-Clause */ #include "ocf/ocf.h" @@ -37,10 +37,6 @@ static void _ocf_engine_ops_complete(struct ocf_request *req, int error) int ocf_engine_ops(struct ocf_request *req) { - struct ocf_cache *cache = req->cache; - - OCF_DEBUG_TRACE(req->cache); - /* Get OCF request - increase reference counter */ ocf_req_get(req); @@ -51,8 +47,9 @@ int ocf_engine_ops(struct ocf_request *req) ocf_submit_volume_req(&req->core->volume, req, _ocf_engine_ops_complete); - ocf_submit_cache_reqs(cache, req, req->rw, 0, req->byte_length, - 1, _ocf_engine_ops_complete); + + /* submit flush to cache device */ + ocf_submit_cache_flush(req, _ocf_engine_ops_complete); /* Put OCF request - decrease reference counter */ ocf_req_put(req); diff --git a/src/utils/utils_io.c b/src/utils/utils_io.c index f969646..f33ffb8 100644 --- a/src/utils/utils_io.c +++ b/src/utils/utils_io.c @@ -224,6 +224,23 @@ static void ocf_submit_volume_req_cmpl(struct ocf_io *io, int error) ocf_io_put(io); } +void ocf_submit_cache_flush(struct ocf_request *req, ocf_req_end_t callback) +{ + uint64_t flags = req->ioi.io.flags; + struct ocf_io *io; + + io = ocf_new_cache_io(req->cache, req->io_queue, 0, 0, OCF_WRITE, 0, + flags); + if (!io) { + callback(req, -OCF_ERR_NO_MEM); + return; + } + + ocf_io_set_cmpl(io, req, callback, ocf_submit_volume_req_cmpl); + + ocf_volume_submit_flush(io); +} + void ocf_submit_cache_reqs(struct ocf_cache *cache, struct ocf_request *req, int dir, uint64_t offset, uint64_t size, unsigned int reqs, ocf_req_end_t callback) diff --git a/src/utils/utils_io.h b/src/utils/utils_io.h index c24c1d9..4419455 100644 --- a/src/utils/utils_io.h +++ b/src/utils/utils_io.h @@ -1,5 +1,5 @@ /* - * Copyright(c) 2012-2021 Intel Corporation + * Copyright(c) 2012-2022 Intel Corporation * SPDX-License-Identifier: BSD-3-Clause */ @@ -64,6 +64,8 @@ void ocf_submit_cache_reqs(struct ocf_cache *cache, struct ocf_request *req, int dir, uint64_t offset, uint64_t size, unsigned int reqs, ocf_req_end_t callback); +void ocf_submit_cache_flush(struct ocf_request *req, ocf_req_end_t callback); + static inline struct ocf_io *ocf_new_cache_io(ocf_cache_t cache, ocf_queue_t queue, uint64_t addr, uint32_t bytes, uint32_t dir, uint32_t io_class, uint64_t flags) From ad2a583f43246e94cfbcc1a9d49e9f3191ade882 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Mon, 16 May 2022 12:46:33 +0200 Subject: [PATCH 4/6] pyocf: test for large I/O Signed-off-by: Adam Rutkowski --- .../functional/tests/engine/test_large_io.py | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/functional/tests/engine/test_large_io.py diff --git a/tests/functional/tests/engine/test_large_io.py b/tests/functional/tests/engine/test_large_io.py new file mode 100644 index 0000000..d6c0ba1 --- /dev/null +++ b/tests/functional/tests/engine/test_large_io.py @@ -0,0 +1,87 @@ +# +# Copyright(c) 2022 Intel Corporation +# SPDX-License-Identifier: BSD-3-Clause +# + +from ctypes import c_int + +from pyocf.types.cache import Cache +from pyocf.types.data import Data +from pyocf.types.core import Core +from pyocf.types.io import IoDir +from pyocf.types.volume import RamVolume, IoFlags +from pyocf.types.volume_core import CoreVolume +from pyocf.utils import Size +from pyocf.types.shared import OcfCompletion + + +def test_large_flush(pyocf_ctx): + cache_device = RamVolume(Size.from_MiB(50)) + core_device = RamVolume(Size.from_MiB(100)) + + cache = Cache.start_on_device(cache_device) + core = Core.using_device(core_device) + cache.add_core(core) + + queue = cache.get_default_queue() + vol = CoreVolume(core, open=True) + + io = vol.new_io(queue, 0, core_device.size.bytes, IoDir.WRITE, 0, IoFlags.FLUSH) + completion = OcfCompletion([("err", c_int)]) + io.callback = completion.callback + data = Data(byte_count=0) + io.set_data(data, 0) + io.submit_flush() + completion.wait() + + assert int(completion.results["err"]) == 0 + + cache.stop() + + +def test_large_discard(pyocf_ctx): + cache_device = RamVolume(Size.from_MiB(50)) + core_device = RamVolume(Size.from_MiB(100)) + + cache = Cache.start_on_device(cache_device) + core = Core.using_device(core_device) + cache.add_core(core) + + queue = cache.get_default_queue() + vol = CoreVolume(core, open=True) + + io = vol.new_io(queue, 0, core_device.size.bytes, IoDir.WRITE, 0, 0) + completion = OcfCompletion([("err", c_int)]) + io.callback = completion.callback + data = Data(byte_count=0) + io.set_data(data, 0) + io.submit_discard() + completion.wait() + + assert int(completion.results["err"]) == 0 + + cache.stop() + + +def test_large_io(pyocf_ctx): + cache_device = RamVolume(Size.from_MiB(50)) + core_device = RamVolume(Size.from_MiB(100)) + + cache = Cache.start_on_device(cache_device) + core = Core.using_device(core_device) + cache.add_core(core) + + queue = cache.get_default_queue() + vol = CoreVolume(core, open=True) + + io = vol.new_io(queue, 0, core_device.size.bytes, IoDir.WRITE, 0, 0) + completion = OcfCompletion([("err", c_int)]) + io.callback = completion.callback + data = Data(byte_count=core_device.size.bytes) + io.set_data(data) + io.submit() + completion.wait() + + assert int(completion.results["err"]) == 0 + + cache.stop() From f0b8815429ade8897cf3051e67265ffb7757574a Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Thu, 19 May 2022 15:01:16 +0200 Subject: [PATCH 5/6] pyocf: reintroduce trace device Signed-off-by: Adam Rutkowski --- tests/functional/pyocf/types/volume.py | 50 ++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tests/functional/pyocf/types/volume.py b/tests/functional/pyocf/types/volume.py index fdee609..5289472 100644 --- a/tests/functional/pyocf/types/volume.py +++ b/tests/functional/pyocf/types/volume.py @@ -473,6 +473,56 @@ class ErrorDevice(Volume): return self.vol.get_copy() +class TraceDevice(Volume): + class IoType(IntEnum): + Data = 1 + Flush = 2 + Discard = 3 + + def __init__(self, vol, trace_fcn=None, uuid=None): + self.vol = vol + super().__init__(uuid) + self.trace_fcn = trace_fcn + + def _trace(self, io, io_type): + submit = True + + if self.trace_fcn: + submit = self.trace_fcn(self, io, io_type) + + return submit + + def do_submit_io(self, io): + submit = self._trace(io, TraceDevice.IoType.Data) + + if submit: + self.vol.do_submit_io(io) + + def do_submit_flush(self, io): + submit = self._trace(io, TraceDevice.IoType.Flush) + + if submit: + self.vol.do_submit_flush(io) + + def get_length(self): + return self.vol.get_length() + + def get_max_io_size(self): + return self.vol.get_max_io_size() + + def do_submit_discard(self, discard): + return self.vol.do_submit_discard(discard) + + def dump(self, offset=0, size=0, ignore=VOLUME_POISON, **kwargs): + return self.vol.dump(offset, size, ignore=ignore, **kwargs) + + def md5(self): + return self.vol.md5() + + def get_copy(self): + return self.vol.get_copy() + + lib = OcfLib.getInstance() lib.ocf_io_get_priv.restype = POINTER(VolumeIoPriv) lib.ocf_io_get_volume.argtypes = [c_void_p] From 28f99ad7a5dc01a459bae61a4bd4013391ff4c12 Mon Sep 17 00:00:00 2001 From: Adam Rutkowski Date: Wed, 18 May 2022 16:54:46 +0200 Subject: [PATCH 6/6] pyocf: flush engine logic test Signed-off-by: Adam Rutkowski --- tests/functional/tests/engine/test_flush.py | 72 +++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tests/functional/tests/engine/test_flush.py diff --git a/tests/functional/tests/engine/test_flush.py b/tests/functional/tests/engine/test_flush.py new file mode 100644 index 0000000..f9fe8d7 --- /dev/null +++ b/tests/functional/tests/engine/test_flush.py @@ -0,0 +1,72 @@ +# +# Copyright(c) 2022-2022 Intel Corporation +# SPDX-License-Identifier: BSD-3-Clause-Clear +# +from ctypes import c_int + +from pyocf.types.cache import Cache +from pyocf.types.data import Data +from pyocf.types.core import Core +from pyocf.types.io import IoDir +from pyocf.types.volume import RamVolume, IoFlags, TraceDevice +from pyocf.types.volume_core import CoreVolume +from pyocf.utils import Size +from pyocf.types.shared import OcfCompletion + + +def test_flush_propagation(pyocf_ctx): + flushes = {} + + pyocf_ctx.register_volume_type(TraceDevice) + + def trace_flush(vol, io, io_type): + nonlocal flushes + + if io_type == TraceDevice.IoType.Flush or int(io.contents._flags) & IoFlags.FLUSH: + if vol.uuid not in flushes: + flushes[vol.uuid] = [] + flushes[vol.uuid].append((io.contents._addr, io.contents._bytes)) + + return True + + cache_device = TraceDevice(RamVolume(Size.from_MiB(50)), trace_fcn=trace_flush) + core_device = TraceDevice(RamVolume(Size.from_MiB(100)), trace_fcn=trace_flush) + + addr = Size.from_MiB(2).B + size = Size.from_MiB(1).B + + cache = Cache.start_on_device(cache_device) + core = Core.using_device(core_device) + cache.add_core(core) + + queue = cache.get_default_queue() + vol = CoreVolume(core, open=True) + + flushes = {} + + io = vol.new_io(queue, addr, size, IoDir.WRITE, 0, IoFlags.FLUSH) + completion = OcfCompletion([("err", c_int)]) + io.callback = completion.callback + data = Data(byte_count=0) + io.set_data(data, 0) + + io.submit_flush() + completion.wait() + + assert int(completion.results["err"]) == 0 + + assert cache_device.uuid in flushes + assert core_device.uuid in flushes + + cache_flushes = flushes[cache_device.uuid] + core_flushes = flushes[core_device.uuid] + + assert len(cache_flushes) == 1 + assert len(core_flushes) == 1 + + assert core_flushes[0] == (addr, size) + + # empty flush expected to be sent to cache device + assert cache_flushes[0] == (0, 0) + + cache.stop()