lkml.org 
[lkml]   [2024]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] comedi: comedi_test: Prevent timers rescheduling during deletion
From
On 13/02/2024 18:10, Ian Abbott wrote:
> The comedi_test devices have a couple of timers (ai_timer and ao_timer)
> that can be started to simulate hardware interrupts. Their expiry
> functions normally reschedule the timer. The driver code calls either
> del_timer_sync() or del_timer() to delete the timers from the queue, but
> does not currently prevent the timers from rescheduling themselves so
> synchronized deletion may be ineffective.
>
> Add a couple of boolean members (one for each timer: ai_timer_enable and
> ao_timer_enable) to the device private data structure to indicate
> whether the timers are allowed to reschedule themselves. Set the member
> to true when adding the timer to the queue, and to false when deleting
> the timer from the queue in the waveform_ai_cancel() and
> waveform_ao_cancel() functions.
>
> The del_timer_sync() function is also called from the waveform_detach()
> function, but the timer enable members will already be set to false when
> that function is called, so no change is needed there.
>
> Fixes: 403fe7f34e33 ("staging: comedi: comedi_test: fix timer race conditions")
> Cc: <stable@vger.kernel.org> # 4.4+
> Signed-off-by: Ian Abbott <abbotti@mev.co.uk>
> ---
> drivers/comedi/drivers/comedi_test.c | 37 +++++++++++++++++++++++++---
> 1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/comedi/drivers/comedi_test.c b/drivers/comedi/drivers/comedi_test.c
> index 30ea8b53ebf8..7fefe0de0bcc 100644
> --- a/drivers/comedi/drivers/comedi_test.c
> +++ b/drivers/comedi/drivers/comedi_test.c
> @@ -87,6 +87,8 @@ struct waveform_private {
> struct comedi_device *dev; /* parent comedi device */
> u64 ao_last_scan_time; /* time of previous AO scan in usec */
> unsigned int ao_scan_period; /* AO scan period in usec */
> + bool ai_timer_enable:1; /* should AI timer be running? */
> + bool ao_timer_enable:1; /* should AO timer be running? */
> unsigned short ao_loopbacks[N_CHANS];
> };
>
> @@ -232,12 +234,18 @@ static void waveform_ai_timer(struct timer_list *t)
> if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg) {
> async->events |= COMEDI_CB_EOA;
> } else {
> + unsigned long flags;
> +
> if (devpriv->ai_convert_time > now)
> time_increment = devpriv->ai_convert_time - now;
> else
> time_increment = 1;
> - mod_timer(&devpriv->ai_timer,
> - jiffies + usecs_to_jiffies(time_increment));
> + spin_lock_irqsave(&dev->spinlock, flags);
> + if (devpriv->ai_timer_enable) {
> + mod_timer(&devpriv->ai_timer,
> + jiffies + usecs_to_jiffies(time_increment));
> + }
> + spin_unlock_irqrestore(&dev->spinlock, flags);

Actually, I should have used a plain old spin_lock() in the above (and
spin_lock_bh() elsewhere in the code) instead of spin_lock_irqsave(),
since there are no hard interrupts involved.

I'll send a v2 patch shortly.

--
-=( Ian Abbott <abbotti@mev.co.uk> || MEV Ltd. is a company )=-
-=( registered in England & Wales. Regd. number: 02862268. )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-


\
 
 \ /
  Last update: 2024-05-27 15:02    [W:0.135 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site