Skip to content

Commit 6e63a68

Browse files
kv2019ilgirdwood
authored andcommitted
audio: component: drop redundant locking in driver list
The component driver list is only modified at FW boot and at runtime when a library is loaded. At boot, module init runs serially on the primary core (Zephyr SYS_INIT at APPLICATION level, before secondary cores are started; .initcall walked on a single core for XTOS). At runtime, registration happens from the IPC thread, which is serialized with only one command processed at a time. These two phases never overlap, as IPC message processing only begins after boot completes, so the list can never be modified concurrently. The lock was also already incoherent: comp_set_adapter_ops() iterate the list without holding the lock, so it provided no real mutual exclusion. Drop the spinlock from comp_register() and comp_unregister(), and from the UUID search in the IPC3 get_drv() reader. Remove the now-unused lock field from struct comp_driver_list and its initialization. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
1 parent 7e26c80 commit 6e63a68

3 files changed

Lines changed: 11 additions & 16 deletions

File tree

src/audio/component.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,23 +45,22 @@ DECLARE_TR_CTX(comp_tr, SOF_UUID(component_uuid), LOG_LEVEL_INFO);
4545
int comp_register(struct comp_driver_info *drv)
4646
{
4747
struct comp_driver_list *drivers = comp_drivers_get();
48-
k_spinlock_key_t key;
4948

50-
key = k_spin_lock(&drivers->lock);
49+
/*
50+
* No locking needed: the driver list is only modified at FW boot,
51+
* where module init runs serially on the primary core, and at
52+
* runtime from the serialized IPC thread (library load). These
53+
* never overlap, so concurrent modification is not possible.
54+
*/
5155
list_item_prepend(&drv->list, &drivers->list);
52-
k_spin_unlock(&drivers->lock, key);
5356

5457
return 0;
5558
}
5659

5760
void comp_unregister(struct comp_driver_info *drv)
5861
{
59-
struct comp_driver_list *drivers = comp_drivers_get();
60-
k_spinlock_key_t key;
61-
62-
key = k_spin_lock(&drivers->lock);
62+
/* see comp_register() on why no locking is needed */
6363
list_item_del(&drv->list);
64-
k_spin_unlock(&drivers->lock, key);
6564
}
6665

6766
int comp_set_adapter_ops(const struct comp_driver *drv, const struct module_interface *ops)
@@ -190,7 +189,6 @@ void sys_comp_init(struct sof *sof)
190189
sof->comp_drivers = platform_shared_get(&cd, sizeof(cd));
191190

192191
list_init(&sof->comp_drivers->list);
193-
k_spinlock_init(&sof->comp_drivers->lock);
194192
}
195193

196194
void comp_get_copy_limits(struct comp_buffer *source,

src/include/sof/audio/component_ext.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
/** \brief Holds list of registered components' drivers */
2727
struct comp_driver_list {
2828
struct list_item list; /**< list of component drivers */
29-
struct k_spinlock lock; /**< list lock */
3029
};
3130

3231
/** \brief Retrieves the component device buffer list. */

src/ipc/ipc3/helper.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp)
8181
struct comp_driver_info *info;
8282
struct sof_ipc_comp_ext *comp_ext;
8383
uintptr_t offset;
84-
k_spinlock_key_t key;
8584

8685
/* do we have extended data ? */
8786
if (!comp->ext_data_length) {
@@ -127,9 +126,10 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp)
127126
goto out;
128127
}
129128

130-
/* search driver list with UUID */
131-
key = k_spin_lock(&drivers->lock);
132-
129+
/*
130+
* search driver list with UUID; no locking needed as the driver
131+
* list is only modified at boot and from the serialized IPC thread
132+
*/
133133
list_for_item(clist, &drivers->list) {
134134
info = container_of(clist, struct comp_driver_info,
135135
list);
@@ -148,8 +148,6 @@ static const struct comp_driver *get_drv(struct sof_ipc_comp *comp)
148148
*(uint32_t *)(&comp_ext->uuid[8]),
149149
*(uint32_t *)(&comp_ext->uuid[12]));
150150

151-
k_spin_unlock(&drivers->lock, key);
152-
153151
out:
154152
if (drv)
155153
tr_dbg(&comp_tr, "get_drv(), found driver type %d, uuid %pU",

0 commit comments

Comments
 (0)