Re: [PATCH] staging: most: dim2: fix a race condition in complete_all_mbos()

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Feb 06, 2026 at 01:02:31AM +0900, Minu Jin wrote:
> The current implementation of complete_all_mbos() repeatedly acquires
> and releases the spinlock in loop. This causes lock contention.
> 
> This patch refactors the function to use list_replace_init(), moving all
> entries to a local list. This removes the loop-based locking approach
> and significantly reduces lock contention.
> 
> Signed-off-by: Minu Jin <s9430939@naver.com>

The subject talks about race conditions but the commit message talks
about reducing lock contention.  It does obviously reduce lock
contention (althought I don't think anyone has benchmarked it to see
if it matters) but does it prevent a race condition?  Let's review:

This complete_all_mbos() function is called when we do a
most_stop_channel() and we ->poison_channel().

The list heads are &hdm_ch->started_list and &hdm_ch->pending_list.  I
feel like if we add something to the list while we are also freeing
items from the list then we are toasted.  In service_done_flag(), we
delete items from the list but deleting items is fine in this context.

We add things to the ->pending_list in enqueue() and
service_done_flag().  We move things from the ->pending_list to the
->started_list in try_start_dim_transfer().  So if any of those three
functions can be run at the same time as complete_all_mbos() we are in
trouble.

The hdm_enqueue_thread() function calls enqueue() until
kthread_should_stop(). The most_stop_channel() function calls
kthread_stop(c->hdm_enqueue_task) before doing the ->poison_channel()
so that's fine.

The service_done_flag() and try_start_dim_transfer() functions are
called from dim2_task_irq().  When do we stop taking interrupts?  To be
honest, I don't know.  I thought we had to call disable_irq()?

So that's the question, when do we disable IRQs in this driver?  I
would have assumed it was in most_stop_channel() but I can't see it,
but I'm also not very familiar with this code.

Let's answer this question and then either add a Fixes tag or say that
there doesn't appear to be a race condition.

regards,
dan carpenter