refactor(kmod): reuse topic_local_id by tomiy-0x62 · Pull Request #1143 · autowarefoundation/agnocast

tomiy-0x62 · 2026-03-02T06:53:24Z

Description

Switched to bitmap-based management for topic_local_id usage.
Enabled ID recycling: topic_local_id is now reused when subscribers or publishers are removed.
Smallest-ID allocation: New subscribers/publishers are assigned the smallest available unused ID.

How was this PR tested?

Autoware (required)
bash scripts/test/e2e_test_1to1.bash (required)
bash scripts/test/e2e_test_2to2.bash (required)
kunit tests (required when modifying the kernel module)
sample application

Notes for reviewers

Version Update Label (Required)

Please add exactly one of the following labels to this PR:

need-major-update: User API breaking changes
need-minor-update: Internal API breaking changes (heaphook/kmod/agnocastlib compatibility)
need-patch-update: Bug fixes and other changes

Important notes:

If you need need-major-update or need-minor-update, please include this in the PR title as well.
- Example: fix(foo)[needs major version update]: bar or feat(baz)[needs minor version update]: qux
After receiving approval from reviewers, add the run-build-test label. The PR can only be merged after the build tests pass.

See CONTRIBUTING.md for detailed versioning rules.

Copilot

Pull request overview

This PR changes how topic_local_id values are managed in the kernel module by replacing a monotonically increasing allocator with a bitmap-based allocator that supports smallest-free-ID assignment and ID recycling.

Changes:

Replace current_pubsub_id with a per-topic bitmap (pubsub_id_map) to track used IDs.
Allocate the smallest available topic_local_id via find_first_zero_bit() for new subscribers/publishers.
Attempt to recycle IDs by clearing the corresponding bitmap bit when subscribers/publishers are removed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-02T08:39:47Z

+  clear_bit(publisher_id, wrapper->topic.pubsub_id_map);
+


clear_bit(publisher_id, wrapper->topic.pubsub_id_map) happens even when pub_info (and/or message entries with en->publisher_id == publisher_id) are intentionally kept because some messages are still referenced by subscribers. This makes the ID reusable while it is still in use, allowing a new publisher to be assigned the same topic_local_id and causing collisions (e.g., find_publisher_info(wrapper, en->publisher_id) can resolve to the wrong publisher or prevent orphaned entries from being GC’d). Only recycle the ID once it is guaranteed that no remaining entries/publisher_info use it (e.g., clear the bit only when pub_info->entries_num == 0 and the publisher_info is actually removed, or track a separate lifetime/epoch for publisher instances).

This is true. You shouldn't clear bit here: should do it as the same time when pub_info is deleted in agnocast_ioctl_remove_publisher and agnocast_ioctl_remove_subscriber.

fix in 5a10a58

You should also add clear_bit for publisher_id in agnocast_ioctl_remove_subscriber.

already exist here https://github.com/tomiy-0x62/agnocast/blob/fa8543510f9553152292744c02ac4e357612d6c1/agnocast_kmod/agnocast_main.c#L2082

publisher_id not subscriber_id.

clear_bit for publisher_id in agnocast_ioctl_remove_subscriber
08b8b95

clear_bit for publisher_id in pre_handler_subscriber_exit
e161844

Copilot · 2026-03-02T08:39:48Z

+  const topic_local_id_t new_id =
+    find_first_zero_bit(wrapper->topic.pubsub_id_map, MAX_TOPIC_LOCAL_ID);
+  if (new_id >= MAX_TOPIC_LOCAL_ID) {
+    dev_warn(
+      agnocast_device,


This introduces smallest-free-ID allocation / recycling for topic_local_id, but the current KUnit suite doesn’t cover reuse scenarios (remove then add should reuse the smallest freed ID; publisher IDs must not be reused while entries with the old publisher_id still exist). Please add KUnit tests covering these new invariants to prevent regressions.

Please try adding the kunit tests as described above.

add kunit test in 97902af

Please add a comment to explicitly show that the test assume the specific algorithm: to allocate the smallest usable id.
We usually don't need the test that assume a specific algorithm, since we also need to change the test when we update it. However, I want the test for this "id reusing feature" case.

test_case_add_publisher_subscriber_too_many verifies whether it is possible to register publishers and subscribers up to MAX_PUBLISHER_NUM and MAX_SUBSCRIBER_NUM.

test_case_add_remove_publisher_subscriber tests the ID reuse feature. It confirms this by registering the maximum number of publishers and subscribers, deleting them all, and then ensuring they can be registered up to the maximum limit again.

Currently, the tests do not include any logic that depends on a specific algorithm.

Could you do the following?

Delete the agnocast_kmod/agnocast_kunit/agnocast_kunit_add_remove_pub_sub.c file and implement the test in the existing test file such as agnocast_kmod/agnocast_kunit/agnocast_kunit_remove_publisher.c.

Implement the test that depend on the specific algorithm so that we can test the reusing feature. (Since we don't have other ways that does it without depending on the specific algorithm.)

Delete the agnocast_kmod/agnocast_kunit/agnocast_kunit_add_remove_pub_sub.c and implement test in agnocast_kmod/agnocast_kunit/agnocast_kunit_remove_publisher.c.
bb651d9

Koichi98 · 2026-03-03T00:44:38Z

@tomiy-0x62
Could you also fix DCO issue, following the direction explained in https://github.com/autowarefoundation/agnocast/pull/1143/checks?check_run_id=65371642276?

Signed-off-by: tomiy <tomiy@tomiylab.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: tomiy <tomiy@tomiylab.com>

Signed-off-by: tomiy <tomiy@tomiylab.com>

Koichi98 · 2026-03-27T01:44:15Z

    goto unlock;
  }

+  clear_bit(subscriber_id, wrapper->topic.pubsub_id_map);


Move this clear_bit and subscriber_id range check above the dev_info( agnocast_device, "Subscriber (id=%d) removed from topic %s.\n", subscriber_id, topic_name);, to make it more natural.

fix in 5a10a58

Koichi98 · 2026-03-27T01:59:52Z

@tomiy-0x62
You should also add clear_bit in pre_handler_publisher_exit and pre_handler_subscriber_exit.

Signed-off-by: tomiy <tomiy@tomiylab.com>

tomiy-0x62 · 2026-03-27T09:59:40Z

@tomiy-0x62 You should also add clear_bit in pre_handler_publisher_exit and pre_handler_subscriber_exit.

fa85435

Signed-off-by: tomiy <tomiy@tomiylab.com>

tomiy-0x62 requested review from Koichi98, atsushi421, kobayu858 and sykwer as code owners March 2, 2026 06:53

Koichi98 requested a review from Copilot March 2, 2026 08:35

Copilot started reviewing on behalf of Koichi98 March 2, 2026 08:35 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Koichi98 assigned tomiy-0x62 Mar 3, 2026

Koichi98 added the need-patch-update Bug fixes and other changes - requires PATCH version update label Mar 3, 2026

tomiy-0x62 and others added 3 commits March 12, 2026 15:49

reuse topic_local_id

fc1151e

Signed-off-by: tomiy <tomiy@tomiylab.com>

fix memory leak

2dddd20

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: tomiy <tomiy@tomiylab.com>

fix memory leak

6c228a6

Signed-off-by: tomiy <tomiy@tomiylab.com>

tomiy-0x62 force-pushed the reuse_topic_local_id branch from 1ba072e to 6c228a6 Compare March 12, 2026 06:50

Merge branch 'main' into reuse_topic_local_id

837ec79

Koichi98 reviewed Mar 27, 2026

View reviewed changes

tomiy-0x62 added 4 commits March 27, 2026 15:51

add agnocast_kunit_add_remove_pub_sub

a49a3ff

Signed-off-by: tomiy <tomiy@tomiylab.com>

implement agnocast_kunit_add_remove_pub_sub and add it to kunit

97902af

Signed-off-by: tomiy <tomiy@tomiylab.com>

change timing of clear pubsub_id_map

5a10a58

Signed-off-by: tomiy <tomiy@tomiylab.com>

clear pubsub_id_map when publihser or subscriber exit

fa85435

Signed-off-by: tomiy <tomiy@tomiylab.com>

tomiy-0x62 added 3 commits March 31, 2026 15:01

format using clangformat

819fb07

Signed-off-by: tomiy <tomiy@tomiylab.com>

add publisher_id range check before clear_bit

0ddcdcd

Signed-off-by: tomiy <tomiy@tomiylab.com>

clear_bit for publisher_id in agnocast_ioctl_remove_subscriber

08b8b95

Signed-off-by: tomiy <tomiy@tomiylab.com>

Koichi98 changed the title ~~reuse topic_local_id~~ refactor: reuse topic_local_id Apr 4, 2026

Koichi98 changed the title ~~refactor: reuse topic_local_id~~ refactor(kmod): reuse topic_local_id Apr 4, 2026

tomiy-0x62 added 3 commits April 8, 2026 18:42

narrow variable scope

ab96e02

Signed-off-by: tomiy <tomiy@tomiylab.com>

fix typo

b97e6e8

Signed-off-by: tomiy <tomiy@tomiylab.com>

implement algorithm-specific tests for topic local ID reuse

bb651d9

Signed-off-by: tomiy <tomiy@tomiylab.com>

clear_bit for publisher_id in pre_handler_subscriber_exit

e161844

Signed-off-by: tomiy <tomiy@tomiylab.com>

Conversation

tomiy-0x62 commented Mar 2, 2026

Description

Related links

How was this PR tested?

Notes for reviewers

Version Update Label (Required)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 2, 2026

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!

Copilot AI Mar 2, 2026

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!

Koichi98 commented Mar 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Koichi98 commented Mar 27, 2026

Uh oh!

tomiy-0x62 commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants