Re: [PATCH] staging: most: video: add comments to mutex and spinlock definitions

[Muhammad Israr]

On Thu, Sep 12, 2026 at 12:19:00AM +0000, Dan Carpenter wrote:
> It's supposed to but it is buggy...  What prevents multiple
> threads from reading comp_vdev_read() at the same time?
> I prefer to keep the warning around until someone fixes the
> code.

Thanks for pointing this out!
I traced through comp_vdev_read(): list_lock (the spinlock --
the mutex field in this struct is unrelated, it's only vdev->lock
used for V4L2 ioctl serialization) is only actually held around
the final list_del() in the read loop. data_ready() and
get_top_mbo(), both called earlier in the same function, read
pending_mbos with no lock held at all. comp_rx_data() (the
rx_completion producer) does take list_lock correctly around its
list_add_tail(), but that only protects against whatever happens
to be holding list_lock at that instant  which today is just
the list_del() call. So nothing stops two threads from both being
inside comp_vdev_read() concurrently and reading/deciding on the
same list state unprotected, which is what you were asking about.

The change I am proposing is to add a dedicated mutex to struct
most_video_dev, held across the whole read() call, so only one
thread is ever inside comp_vdev_read() at a time. list_lock still
wraps the actual list touches (checking for an empty list and
picking the head entry, and the existing list_del()), matching
what comp_rx_data() already does; copy_to_user() stays outside
any lock since it can fault. The mutex handles thread-vs-thread
serialization, list_lock keeps handling reader-vs-rx_completion
synchronization.

Does that match what you had in mind, or would you take a
different approach?

I will send the race fix on its own, and once the locking is
accurate I will send the follow-up comment patch.

regards,
Muhammad Israr