The V4L2 subdevs have managed without centralized locking for the state
(previously pad_config), as the TRY state is supposedly safe (although I
believe two TRY ioctls for the same fd would race), and the ACTIVE
state, and its locking, is managed by the drivers internally.
We now have ACTIVE state in a centralized position, and need locking.
Strictly speaking the locking is only needed for new drivers that use
the new state, as the current drivers continue behaving as they used to.
Add a mutex to the struct v4l2_subdev_state, along with a few helper
functions for locking/unlocking.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
---
drivers/media/v4l2-core/v4l2-subdev.c | 43 +++++++++++++++++----
include/media/v4l2-subdev.h | 55 +++++++++++++++++++++++++--
2 files changed, 88 insertions(+), 10 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index b3637cddca58..b1e65488210d 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -26,9 +26,11 @@
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
static int subdev_fh_init(struct v4l2_subdev_fh *fh, struct v4l2_subdev *sd)
{
+ static struct lock_class_key __key;
struct v4l2_subdev_state *state;
- state = v4l2_alloc_subdev_state(sd, V4L2_SUBDEV_FORMAT_TRY);
+ state = __v4l2_alloc_subdev_state(sd, V4L2_SUBDEV_FORMAT_TRY,
+ "v4l2_subdev_fh->state", &__key);
if (IS_ERR(state))
return PTR_ERR(state);
@@ -924,8 +926,10 @@ int v4l2_subdev_link_validate(struct media_link *link)
EXPORT_SYMBOL_GPL(v4l2_subdev_link_validate);
struct v4l2_subdev_state *
-v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
- enum v4l2_subdev_format_whence which)
+__v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
+ enum v4l2_subdev_format_whence which,
+ const char *lock_name,
+ struct lock_class_key *lock_key)
{
struct v4l2_subdev_state *state;
int ret;
@@ -934,6 +938,8 @@ v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
if (!state)
return ERR_PTR(-ENOMEM);
+ __mutex_init(&state->lock, lock_name, lock_key);
+
state->which = which;
if (sd->entity.num_pads) {
@@ -960,13 +966,15 @@ v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
return ERR_PTR(ret);
}
-EXPORT_SYMBOL_GPL(v4l2_alloc_subdev_state);
+EXPORT_SYMBOL_GPL(__v4l2_alloc_subdev_state);
void v4l2_free_subdev_state(struct v4l2_subdev_state *state)
{
if (!state)
return;
+ mutex_destroy(&state->lock);
+
kvfree(state->pads);
kfree(state);
}
@@ -1001,11 +1009,12 @@ void v4l2_subdev_notify_event(struct v4l2_subdev *sd,
}
EXPORT_SYMBOL_GPL(v4l2_subdev_notify_event);
-int v4l2_subdev_alloc_state(struct v4l2_subdev *sd)
+int __v4l2_subdev_alloc_state(struct v4l2_subdev *sd, const char *name,
+ struct lock_class_key *key)
{
struct v4l2_subdev_state *state;
- state = v4l2_alloc_subdev_state(sd, V4L2_SUBDEV_FORMAT_ACTIVE);
+ state = __v4l2_alloc_subdev_state(sd, V4L2_SUBDEV_FORMAT_ACTIVE, name, key);
if (IS_ERR(state))
return PTR_ERR(state);
@@ -1013,7 +1022,7 @@ int v4l2_subdev_alloc_state(struct v4l2_subdev *sd)
return 0;
}
-EXPORT_SYMBOL_GPL(v4l2_subdev_alloc_state);
+EXPORT_SYMBOL_GPL(__v4l2_subdev_alloc_state);
void v4l2_subdev_free_state(struct v4l2_subdev *sd)
{
@@ -1021,3 +1030,23 @@ void v4l2_subdev_free_state(struct v4l2_subdev *sd)
sd->state = NULL;
}
EXPORT_SYMBOL_GPL(v4l2_subdev_free_state);
+
+struct v4l2_subdev_state *v4l2_subdev_lock_active_state(struct v4l2_subdev *sd)
+{
+ mutex_lock(&sd->state->lock);
+
+ return sd->state;
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_lock_active_state);
+
+void v4l2_subdev_lock_state(struct v4l2_subdev_state *state)
+{
+ mutex_lock(&state->lock);
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_lock_state);
+
+void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state)
+{
+ mutex_unlock(&state->lock);
+}
+EXPORT_SYMBOL_GPL(v4l2_subdev_unlock_state);
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index 5ec78ffda4f5..52a725281b23 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -655,6 +655,7 @@ struct v4l2_subdev_pad_config {
/**
* struct v4l2_subdev_state - Used for storing subdev state information.
*
+ * @lock: mutex for the state
* @which: state type (from enum v4l2_subdev_format_whence)
* @pads: &struct v4l2_subdev_pad_config array
*
@@ -663,6 +664,7 @@ struct v4l2_subdev_pad_config {
* %V4L2_SUBDEV_FORMAT_ACTIVE it is safe to pass %NULL.
*/
struct v4l2_subdev_state {
+ struct mutex lock;
u32 which;
struct v4l2_subdev_pad_config *pads;
};
@@ -1147,9 +1149,18 @@ int v4l2_subdev_link_validate(struct media_link *link);
*
* Must call v4l2_free_subdev_state() when state is no longer needed.
*/
+#define v4l2_alloc_subdev_state(sd, which) \
+ ({ \
+ static struct lock_class_key __key; \
+ const char *name = KBUILD_BASENAME \
+ ":" __stringify(__LINE__) ":sd->state->lock"; \
+ __v4l2_alloc_subdev_state(sd, which, name, &__key); \
+ })
+
struct v4l2_subdev_state *
-v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
- enum v4l2_subdev_format_whence which);
+__v4l2_alloc_subdev_state(struct v4l2_subdev *sd,
+ enum v4l2_subdev_format_whence which,
+ const char *lock_name, struct lock_class_key *key);
/**
* v4l2_free_subdev_state - free a v4l2_subdev_state
@@ -1234,7 +1245,16 @@ void v4l2_subdev_notify_event(struct v4l2_subdev *sd,
*
* Must call v4l2_subdev_free_state() when the state is no longer needed.
*/
-int v4l2_subdev_alloc_state(struct v4l2_subdev *sd);
+#define v4l2_subdev_alloc_state(sd) \
+ ({ \
+ static struct lock_class_key __key; \
+ const char *name = KBUILD_BASENAME \
+ ":" __stringify(__LINE__) ":sd->state->lock"; \
+ __v4l2_subdev_alloc_state(sd, name, &__key); \
+ })
+
+int __v4l2_subdev_alloc_state(struct v4l2_subdev *sd, const char *name,
+ struct lock_class_key *key);
/**
* v4l2_subdev_free_state() - Free the active subdev state for subdevice
@@ -1258,4 +1278,33 @@ v4l2_subdev_get_active_state(struct v4l2_subdev *sd)
return sd->state;
}
+/**
+ * v4l2_subdev_lock_active_state() - Lock and return the active subdev state for subdevice
+ * @sd: The subdevice
+ *
+ * Return the locked active state for the subdevice, or NULL if the subdev
+ * does not support active state.
+ *
+ * Must be unlocked with v4l2_subdev_unlock_state() after use.
+ */
+struct v4l2_subdev_state *v4l2_subdev_lock_active_state(struct v4l2_subdev *sd);
+
+/**
+ * v4l2_subdev_lock_state() - Lock the subdev state
+ * @state: The subdevice state
+ *
+ * Lock the given subdev state.
+ *
+ * Must be unlocked with v4l2_subdev_unlock_state() after use.
+ */
+void v4l2_subdev_lock_state(struct v4l2_subdev_state *state);
+
+/**
+ * v4l2_subdev_unlock_state() - Unlock the subdev state
+ * @state: The subdevice state
+ *
+ * Unlock the given subdev state.
+ */
+void v4l2_subdev_unlock_state(struct v4l2_subdev_state *state);
+
#endif
--
2.25.1
