diff options
| author |   Marcelo Diop-Gonzalez <marcgonzalez@google.com> | 2020-02-12 13:43:32 -0500 |
|---|---|---|
| committer |   Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2020-02-12 13:40:43 -0800 |
| commit | 3c27a36f2711880de5e6629fbba71bfdbbf47ceb (patch) | |
| tree | 883b41127d16f136c4e392e7f7d28e5848e45fb5 /drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h | |
| parent | staging: gasket: unify multi-line string (diff) | |
| download | linux-dev-3c27a36f2711880de5e6629fbba71bfdbbf47ceb.tar.xz linux-dev-3c27a36f2711880de5e6629fbba71bfdbbf47ceb.zip | |
staging: vc04_services: use kref + RCU to reference count services
Currently reference counts are implemented by locking service_spinlock
and then incrementing the service's ->ref_count field, calling
kfree() when the last reference has been dropped. But at the same
time, there's code in multiple places that dereferences pointers
to services without having a reference, so there could be a race there.
It should be possible to avoid taking any lock in unlock_service()
or service_release() because we are setting a single array element
to NULL, and on service creation, a mutex is locked before looking
for a NULL spot to put the new service in.
Using a struct kref and RCU-delaying the freeing of services fixes
this race condition while still making it possible to skip
grabbing a reference in many places. Also it avoids the need to
acquire a single spinlock when e.g. taking a reference on
state->services[i] when somebody else is in the middle of taking
a reference on state->services[j].
Signed-off-by: Marcelo Diop-Gonzalez <marcgonzalez@google.com>
Link: https://lore.kernel.org/r/3bf6f1ec6ace64d7072025505e165b8dd18b25ca.1581532523.git.marcgonzalez@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h')
| -rw-r--r-- | drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h | 12 |
1 files changed, 8 insertions, 4 deletions
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index 604d0c330819..30e4965c7666 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -7,6 +7,8 @@ #include <linux/mutex.h> #include <linux/completion.h> #include <linux/kthread.h> +#include <linux/kref.h> +#include <linux/rcupdate.h> #include <linux/wait.h> #include "vchiq_cfg.h" @@ -251,7 +253,8 @@ struct vchiq_slot_info { struct vchiq_service { struct vchiq_service_base base; unsigned int handle; - unsigned int ref_count; + struct kref ref_count; + struct rcu_head rcu; int srvstate; vchiq_userdata_term userdata_term; unsigned int localport; @@ -464,7 +467,7 @@ struct vchiq_state { int error_count; } stats; - struct vchiq_service *services[VCHIQ_MAX_SERVICES]; + struct vchiq_service __rcu *services[VCHIQ_MAX_SERVICES]; struct vchiq_service_quota service_quotas[VCHIQ_MAX_SERVICES]; struct vchiq_slot_info slot_info[VCHIQ_MAX_SLOTS]; @@ -545,12 +548,13 @@ request_poll(struct vchiq_state *state, struct vchiq_service *service, static inline struct vchiq_service * handle_to_service(unsigned int handle) { + int idx = handle & (VCHIQ_MAX_SERVICES - 1); struct vchiq_state *state = vchiq_states[(handle / VCHIQ_MAX_SERVICES) & (VCHIQ_MAX_STATES - 1)]; + if (!state) return NULL; - - return state->services[handle & (VCHIQ_MAX_SERVICES - 1)]; + return rcu_dereference(state->services[idx]); } extern struct vchiq_service * |