Hillf Danton <hdanton@xxxxxxxx> 于2023年2月21日周二 19:15写道：
>
> Feel free to add info like how this fix was tested.
Hi Hillf,
Sorry for my unclear description. I don't have the device to test. I
learn it from someone else's experience.
> >
> > Fix it by adding del_timer_sync invoking to the remove function.
> >
> > cpu0 cpu1
> > bttv_probe
> > ->timer_setup
> > ->bttv_set_dma
> > ->mod_timer;
> > bttv_remove
> > ->kfree(btv);
> > ->bttv_irq_timeout
> > ->USE btv
> >
> > Signed-off-by: Zheng Wang <zyytlz.wz@xxxxxxx>
> > ---
> > drivers/media/pci/bt8xx/bttv-driver.c | 1 +
> > drivers/media/pci/bt8xx/bttv-risc.c | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c
> > index d40b537f4e98..24ba5729969d 100644
> > --- a/drivers/media/pci/bt8xx/bttv-driver.c
> > +++ b/drivers/media/pci/bt8xx/bttv-driver.c
> > @@ -4248,6 +4248,7 @@ static void bttv_remove(struct pci_dev *pci_dev)
> >
> > /* free resources */
> > free_irq(btv->c.pci->irq,btv);
> > + del_timer_sync(&btv->timeout);
> > iounmap(btv->bt848_mmio);
> > release_mem_region(pci_resource_start(btv->c.pci,0),
> > pci_resource_len(btv->c.pci,0));
> > diff --git a/drivers/media/pci/bt8xx/bttv-risc.c b/drivers/media/pci/bt8xx/bttv-risc.c
> > index 32fa4a7fe76f..ada469198645 100644
> > --- a/drivers/media/pci/bt8xx/bttv-risc.c
> > +++ b/drivers/media/pci/bt8xx/bttv-risc.c
> > @@ -481,7 +481,7 @@ bttv_set_dma(struct bttv *btv, int override)
> > if (btv->curr.frame_irq || btv->loop_irq || btv->cvbi) {
> > mod_timer(&btv->timeout, jiffies+BTTV_TIMEOUT);
> > } else {
> > - del_timer(&btv->timeout);
> > + del_timer_sync(&btv->timeout);
> > }
> > btv->main.cpu[RISC_SLOT_LOOP] = cpu_to_le32(cmd);
> >
> > --
> > 2.25.1
>
> This patch adds chance for deadlock by replacing del_timer() with del_timer_sync(), right?
>
> cpu 0 cpu 2
> --- ---
> bttv_read() bttv_irq_timeout()
> bttv_reinit_bt848()
> spin_lock_irqsave(&btv->s_lock,flags);
>
> spin_lock_irqsave(&btv->s_lock,flags);
> deadlock;
> btv->errors=0;
> bttv_set_dma(btv,0);
> del_timer_sync(&btv->timeout);
Yes, it is. Thanks for pointing that out. Could you please give some
advice about the fix?
Best regards,
Zheng
