Hi Sergey, On Thu, Oct 24, 2024 at 2:13 PM Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> wrote: > > On (24/10/24 13:58), Sergey Senozhatsky wrote: > > Date: Thu, 24 Oct 2024 13:58:36 +0900 > > From: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> > > To: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx> > > Cc: Stanimir Varbanov <stanimir.k.varbanov@xxxxxxxxx>, Vikash Garodia > > <quic_vgarodia@xxxxxxxxxxx>, Bryan O'Donoghue > > <bryan.odonoghue@xxxxxxxxxx>, linux-media@xxxxxxxxxxxxxxx, > > linux-kernel@xxxxxxxxxxxxxxx > > Subject: Re: [PATCH 2/2] media: venus: sync with threaded IRQ during inst > > destruction > > Message-ID: <20241024045836.GJ1279924@xxxxxxxxxx> > > > > On (24/10/23 14:24), Sergey Senozhatsky wrote: > > > Guard inst destruction (both dec and enc) with hard and threaded > > > IRQ synchronization. > > > > Folks, please ignore this patch. Stand by for v2. > > I think it probably should be something like this (both for dec and > enc). > > --- > > @@ -1538,9 +1538,25 @@ static int venc_close(struct file *file) > > venc_pm_get(inst); > > + /* > + * First, remove the inst from the ->instances list, so that > + * to_instance() will return NULL. > + */ > + hfi_session_destroy(inst); > + /* > + * Second, make sure we don't have IRQ/IRQ-thread currently running or > + * pending execution (disable_irq() calls synchronize_irq()), which > + * can race with the inst destruction. > + */ > + disable_irq(inst->core->irq); > + /* > + * Lastly, inst is gone from the core->instances list and we don't > + * have running/pending IRQ/IRQ-thread, proceed with the destruction > + */ > + enable_irq(inst->core->irq); > + Thanks a lot for looking into this. Wouldn't it be enough to just call synchronize_irq() at this point, since the instance was removed from the list already? I guess the question is if that's the only way the interrupt handler can get hold of the instance. Best, Tomasz > v4l2_m2m_ctx_release(inst->m2m_ctx); > v4l2_m2m_release(inst->m2m_dev); > - hfi_session_destroy(inst); > v4l2_fh_del(&inst->fh); > v4l2_fh_exit(&inst->fh); > venc_ctrl_deinit(inst); >
