OCF module v2 #37

rafalste · 2025-04-07T08:45:21Z

Change-Id: I58f0fa973cc78b1c7bfefd9ffd783a6f752baba3

module/bdev/ocf/vbdev_ocf_cache.c

module/bdev/ocf/vbdev_ocf_core.c

mmichal10 · 2025-08-28T06:29:22Z

module/bdev/ocf/vbdev_ocf.c

 	}

-	return status;
+	/* Increment queue refcount to prevent destroying management queue after cache device detach. */
+	ocf_queue_get(((struct vbdev_ocf_cache *)ocf_cache_get_priv(cache))->cache_mngt_q);


I think the mngt queue refcnt should be incremented once, on cache start. Incrementing it here feels like a workaround

The problem turned out to be in the context cleaner refcount. Fixed.

module/bdev/ocf/vbdev_ocf.c

mmichal10 · 2025-08-28T07:00:00Z

module/bdev/ocf/vbdev_ocf_core.c

+	SPDK_DEBUGLOG(vbdev_ocf, "OCF vbdev '%s': destroying IO channel\n",
+		      spdk_bdev_get_name(&(((struct vbdev_ocf_core *)ocf_core_get_priv(core))->ocf_vbdev)));
+
+	ocf_queue_put(ch_destroy_ctx->queue);


The core_bdev iochannel, the cache_bdev iochannel, the poller and the queue are all owned by vbdev_iochannel. I think you should decrement all their refcounter here on vbdev_channel_destroy just as you're creating them on vbdev_channel_create.

Since the channel context with all of those you mention is saved in queue priv, this decrementing will be done in vbdev_ocf_core_io_queue_stop() when the queue is destroyed.

mmichal10 · 2025-08-28T07:56:21Z

module/bdev/ocf/volume.c

+vbdev_ocf_base_detach(struct vbdev_ocf_base *base)
+{
+	if (base->thread && base->thread != spdk_get_thread()) {
+		spdk_thread_send_msg(base->thread, _base_detach, base);


The error returned by spdk_thread_send_msg() shouldn't be ignored

Almost all usage of spdk_thread_send_msg() in all other modules look like this. But you're right - why not use it if it's there. Fixed.

mmichal10 · 2025-08-28T09:23:23Z

module/bdev/ocf/ctx.c

+	ocf_queue_t queue = ctx;
+	int i, queue_runs;
+
+	queue_runs = spdk_min(ocf_queue_pending_io(queue), VBDEV_OCF_QUEUE_RUN_MAX);


Why is there a limit of queue runs?

To not run the queue for to long and let it be rescheduled if needed.

mmichal10 · 2025-08-28T09:36:12Z

module/bdev/ocf/vbdev_ocf.c

-		return;
+	SPDK_DEBUGLOG(vbdev_ocf, "OCF cache '%s': module stop visit\n", ocf_cache_get_name(cache));
+
+	/* Increment cache refcount to not destroy cache structs before destroying all cores. */


This sounds like a workaround. I think we should have a proper management of cache lifetime.

Already removed this bit because it is actually not needed anymore since cache stop is not triggered asynchronously with cores detach any more.
I found a better solution for this in later commits and it is taken care of by this check:
ocf_cache_get_core_count(cache) == ocf_cache_get_core_inactive_count(cache)

mmichal10 · 2025-08-28T09:40:54Z

module/bdev/ocf/vbdev_ocf.c

+	// impossible to be true ?
+	if (!core) {
+		SPDK_ERRLOG("OCF vbdev '%s': failed to submit IO - no OCF core device\n",
+			    spdk_bdev_get_name(bdev_io->bdev));
+		spdk_bdev_io_complete(bdev_io, SPDK_BDEV_IO_STATUS_FAILED);
+		return;
 	}


If it's impossible it could be a BUG_ON

This situation is not possible to occur, so I removed this check. Nevertheless, added an assert here just to be sure.

github-actions · 2025-12-17T13:46:45Z

SPDK manages code review in https://review.spdk.io. This PR will be closed automatically.

github-actions · 2025-12-20T13:37:43Z

SPDK manages code review in https://review.spdk.io. This PR will be closed automatically.

Change-Id: Icf9704d338fe8fe5d3f6ef8eb5f9255af4a649d2 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I58f0fa973cc78b1c7bfefd9ffd783a6f752baba3 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ie8d756c4374f5fb75e39a885b203a6b6883d1bf5 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I82aae275e79444afbda292bd0cc8211d18fd5c0e Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I74ef4446df4f51d662b31e2774a8d1c7f1c11df0 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I6cd3262aed2b309929df68d24fac5b431ecfc006 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ie509dc0f48e76cd9c01cb7e07ef8698dfa3bab14 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I91828704921fb74ff538ba37e6f6a5a8cbd218bf Signed-off-by: Rafal Stefanowski <[email protected]>

- check cache/core block size - merge cache stop paths - unify adding cores from waitlist - error handling - comments - cleanup Change-Id: Ib111089a9e5fe1ec155487db3b2c4541d1ce9520 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I1d4ecf9555f9996d236c4985ce460a8839b02e2f Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I0049cda213a5d037819b877cd1279286a1a68841 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ia6aa94a2f352d1f9178958dd34e7e069ea2bc3d9 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ib36ae38a03d1bca9b2595a78b7cc162ab787ebe8 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I70e3d6470dc3f0c7a6a493472746ccdc3bd3c6e7 Signed-off-by: Rafal Stefanowski <[email protected]>

- update cache IO channels after device detach/attach - fix management queue refcount - fix IO channels double free - handle spdk_thread_send_msg() errors - do not allow using OCF core as base bdev for another OCF core - remove redundant checks - remove cluttering logs Change-Id: I34f2aceb434d5752c787ae6071cce3930bc44575 Signed-off-by: Rafal Stefanowski <[email protected]>

- current module config - generic bdev info - generic bdev stats - OCF stats Change-Id: If63774a399902caaa50c09ef2e89a9837e5db95a Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I106829e59cdf145ed62998c6b101d4efd7389821 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ib43ab22b3fa457e624deae912b3c587bdf6b0648 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Id6ddfa4042427715f7927c7d66fb98ee9904881c Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I7242902219bcb5604df51f8a06feb50e86426541 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ia187bee78d1e1078eefcb49ebcaf73ffd601f479 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I3753edb1643a4465c9768e6a8a13cd4c21cc6443 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I488ec6c802299aba39f38743cad12a429ece066e Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I107349f86aef598b5ec112d1a6df5de20ea4a2c3 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ie01d40cebf469df6d84f5689b2767f6aa7f3721c Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I915d5af1cce0ce38bca45462a7b66d14ce8be8e7 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I3607fd8a261755440fdd2948a027a37aaf66d67e Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: If00f029f084e5f500af7d7e6524cf33193fa1860 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ib038d093689c45fade8515861b50cb2b30f5601d Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I323302f657e0d943acd3b4d4e4c8bd3efd4931d0 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Iaeaf9a9af3c82d908efddf5947a55bf8b2f0fcc2 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: Ibba93fc9803babcc9c3055aeb223825c580f7743 Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I80e5bc492f9025a13ba4cdc7ca12676bb0ef3f8f Signed-off-by: Rafal Stefanowski <[email protected]>

Change-Id: I03748efcc653d49fe561731046ff236fcd37314a Signed-off-by: Rafal Stefanowski <[email protected]>

rafalste force-pushed the ocf_module_refactor branch from 1cda3c3 to 9cc8288 Compare April 7, 2025 09:49

mmichal10 reviewed Apr 8, 2025

View reviewed changes

module/bdev/ocf/vbdev_ocf_cache.c Outdated Show resolved Hide resolved

module/bdev/ocf/vbdev_ocf_cache.c Outdated Show resolved Hide resolved

rafalste force-pushed the ocf_module_refactor branch 2 times, most recently from 217180c to a06263a Compare July 25, 2025 15:06

mmichal10 reviewed Aug 28, 2025

View reviewed changes

rafalste force-pushed the ocf_module_refactor branch 4 times, most recently from 46b141d to b5510ef Compare September 19, 2025 13:31

rafalste force-pushed the ocf_module_refactor branch 2 times, most recently from 266d53d to 43e7e51 Compare October 7, 2025 15:58

rafalste force-pushed the ocf_module_refactor branch 2 times, most recently from e4b03ac to 9ba18bf Compare October 14, 2025 15:20

rafalste force-pushed the ocf_module_refactor branch 2 times, most recently from 71f7311 to 472ff8a Compare October 31, 2025 14:50

github-actions bot added the Stale label Dec 17, 2025

github-actions bot closed this Dec 18, 2025

rafalste reopened this Dec 18, 2025

rafalste removed the Stale label Dec 18, 2025

github-actions bot added the Stale label Dec 20, 2025

Rafal Stefanowski added 8 commits December 20, 2025 16:02

Remove old OCF module

26e24a6

Change-Id: Icf9704d338fe8fe5d3f6ef8eb5f9255af4a649d2 Signed-off-by: Rafal Stefanowski <[email protected]>

OCF module v2

42ff735

Change-Id: I58f0fa973cc78b1c7bfefd9ffd783a6f752baba3 Signed-off-by: Rafal Stefanowski <[email protected]>

OCF module: move data to OCF structs

b54fa70

Change-Id: Ie8d756c4374f5fb75e39a885b203a6b6883d1bf5 Signed-off-by: Rafal Stefanowski <[email protected]>

fix memleaks

a5b59d2

Change-Id: I82aae275e79444afbda292bd0cc8211d18fd5c0e Signed-off-by: Rafal Stefanowski <[email protected]>

Implement bdev examine and hotremove

cff45c1

Change-Id: I74ef4446df4f51d662b31e2774a8d1c7f1c11df0 Signed-off-by: Rafal Stefanowski <[email protected]>

Update OCF to 25.03

83cc3ae

Change-Id: I6cd3262aed2b309929df68d24fac5b431ecfc006 Signed-off-by: Rafal Stefanowski <[email protected]>

Implement cache load

aef6dd1

Change-Id: Ie509dc0f48e76cd9c01cb7e07ef8698dfa3bab14 Signed-off-by: Rafal Stefanowski <[email protected]>

Management tests

a9039b9

Change-Id: I91828704921fb74ff538ba37e6f6a5a8cbd218bf Signed-off-by: Rafal Stefanowski <[email protected]>

Rafal Stefanowski added 26 commits December 20, 2025 16:02

Multiple fixes

c4e6460

- check cache/core block size - merge cache stop paths - unify adding cores from waitlist - error handling - comments - cleanup Change-Id: Ib111089a9e5fe1ec155487db3b2c4541d1ce9520 Signed-off-by: Rafal Stefanowski <[email protected]>

Parameterize tests

1dc8f8e

Change-Id: I1d4ecf9555f9996d236c4985ce460a8839b02e2f Signed-off-by: Rafal Stefanowski <[email protected]>

Implement configuration change RPCs

50b5214

Change-Id: I0049cda213a5d037819b877cd1279286a1a68841 Signed-off-by: Rafal Stefanowski <[email protected]>

Configuration change tests

6ff1a71

Change-Id: Ia6aa94a2f352d1f9178958dd34e7e069ea2bc3d9 Signed-off-by: Rafal Stefanowski <[email protected]>

Various improvements

3abb6cd

Change-Id: Ib36ae38a03d1bca9b2595a78b7cc162ab787ebe8 Signed-off-by: Rafal Stefanowski <[email protected]>

Implement statistics

936d755

Change-Id: I70e3d6470dc3f0c7a6a493472746ccdc3bd3c6e7 Signed-off-by: Rafal Stefanowski <[email protected]>

Implement info ops + tests

e2307bc

- current module config - generic bdev info - generic bdev stats - OCF stats Change-Id: If63774a399902caaa50c09ef2e89a9837e5db95a Signed-off-by: Rafal Stefanowski <[email protected]>

Address all of the remaining in-code comments

1e808cf

Change-Id: I106829e59cdf145ed62998c6b101d4efd7389821 Signed-off-by: Rafal Stefanowski <[email protected]>

Fix formatting

7d84647

Change-Id: Ib43ab22b3fa457e624deae912b3c587bdf6b0648 Signed-off-by: Rafal Stefanowski <[email protected]>

Update documentation

445e59e

Change-Id: Id6ddfa4042427715f7927c7d66fb98ee9904881c Signed-off-by: Rafal Stefanowski <[email protected]>

Prevent running any calls when module is stopping

e1f8eff

Change-Id: I7242902219bcb5604df51f8a06feb50e86426541 Signed-off-by: Rafal Stefanowski <[email protected]>

Do not print uninitialized core info

d5c0585

Change-Id: Ia187bee78d1e1078eefcb49ebcaf73ffd601f479 Signed-off-by: Rafal Stefanowski <[email protected]>

Generate UUIDs for registered bdevs

74ea79c

Change-Id: I3753edb1643a4465c9768e6a8a13cd4c21cc6443 Signed-off-by: Rafal Stefanowski <[email protected]>

Remove old tests

62630ab

Change-Id: I488ec6c802299aba39f38743cad12a429ece066e Signed-off-by: Rafal Stefanowski <[email protected]>

Refactor new tests

cbbefaf

Change-Id: I107349f86aef598b5ec112d1a6df5de20ea4a2c3 Signed-off-by: Rafal Stefanowski <[email protected]>

Add integrity tests

6fcbbb9

Change-Id: Ie01d40cebf469df6d84f5689b2767f6aa7f3721c Signed-off-by: Rafal Stefanowski <[email protected]>

Read trylock cache in bdev_ocf_get_bdevs

0eaa11a

Change-Id: I915d5af1cce0ce38bca45462a7b66d14ce8be8e7 Signed-off-by: Rafal Stefanowski <[email protected]>

Fix race condition when adding cores in examine

3f2a8b8

Change-Id: I3607fd8a261755440fdd2948a027a37aaf66d67e Signed-off-by: Rafal Stefanowski <[email protected]>

Move cache IO channel update before attaching new device to OCF

ca71913

Change-Id: If00f029f084e5f500af7d7e6524cf33193fa1860 Signed-off-by: Rafal Stefanowski <[email protected]>

Make sure selected core still exists after taking a cache lock

6adce76

Change-Id: Ib038d093689c45fade8515861b50cb2b30f5601d Signed-off-by: Rafal Stefanowski <[email protected]>

Add stress tests

0680837

Change-Id: I323302f657e0d943acd3b4d4e4c8bd3efd4931d0 Signed-off-by: Rafal Stefanowski <[email protected]>

Make a copy of core context when dumping core info

2f21b83

Change-Id: Iaeaf9a9af3c82d908efddf5947a55bf8b2f0fcc2 Signed-off-by: Rafal Stefanowski <[email protected]>

Make a copy of bdev name given in RPC call

a2b2c13

Change-Id: Ibba93fc9803babcc9c3055aeb223825c580f7743 Signed-off-by: Rafal Stefanowski <[email protected]>

Add dirty_ratio_inertia cleaning param

91ba30a

Change-Id: I80e5bc492f9025a13ba4cdc7ca12676bb0ef3f8f Signed-off-by: Rafal Stefanowski <[email protected]>

Update OCF

4a15191

Change-Id: I03748efcc653d49fe561731046ff236fcd37314a Signed-off-by: Rafal Stefanowski <[email protected]>

rafalste force-pushed the ocf_module_refactor branch from 472ff8a to 4a15191 Compare December 20, 2025 15:17

rafalste removed the Stale label Dec 20, 2025

OCF module v2 #37

Are you sure you want to change the base?

OCF module v2 #37

Uh oh!

Conversation

rafalste commented Apr 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 17, 2025

Uh oh!

github-actions bot commented Dec 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants