The _rtw_init_recv_priv() initializes precvpriv->signal_stat_timer and
sets it's timeout interval to 1000 ms. But _rtw_free_recv_priv()
doesn't cancel the timer and we need to explicitly call
_cancel_timer_ex() after we call _rtw_free_recv_priv() to cancel the
timer.
Call _cancel_timer_ex() from inside _rtw_free_recv_priv() as every init
function needs a matching free function.
Signed-off-by: Vihas Makwana <[email protected]>
---
drivers/staging/r8188eu/core/rtw_recv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
index d77d98351..61308eb39 100644
--- a/drivers/staging/r8188eu/core/rtw_recv.c
+++ b/drivers/staging/r8188eu/core/rtw_recv.c
@@ -103,6 +103,7 @@ void _rtw_free_recv_priv(struct recv_priv *precvpriv)
vfree(precvpriv->pallocated_frame_buf);
rtl8188eu_free_recv_priv(padapter);
+ _cancel_timer_ex(&precvpriv->signal_stat_timer);
}
struct recv_frame *_rtw_alloc_recvframe(struct __queue *pfree_recv_queue)
--
2.30.2
On Wed, Mar 09, 2022 at 02:25:09AM +0530, Vihas Makwana wrote:
> The _rtw_init_recv_priv() initializes precvpriv->signal_stat_timer and
> sets it's timeout interval to 1000 ms. But _rtw_free_recv_priv()
> doesn't cancel the timer and we need to explicitly call
> _cancel_timer_ex() after we call _rtw_free_recv_priv() to cancel the
> timer.
> Call _cancel_timer_ex() from inside _rtw_free_recv_priv() as every init
> function needs a matching free function.
>
>
> Signed-off-by: Vihas Makwana <[email protected]>
> ---
> drivers/staging/r8188eu/core/rtw_recv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> index d77d98351..61308eb39 100644
> --- a/drivers/staging/r8188eu/core/rtw_recv.c
> +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> @@ -103,6 +103,7 @@ void _rtw_free_recv_priv(struct recv_priv *precvpriv)
> vfree(precvpriv->pallocated_frame_buf);
>
> rtl8188eu_free_recv_priv(padapter);
> + _cancel_timer_ex(&precvpriv->signal_stat_timer);
> }
*If* then timer_setup() belongs in _rtw_init_recv_priv() then this is
where the _cancel_timer_ex() belongs, yes. But what about if the devs
hid it in a different wrong place?
Right the del_timer is in rtw_cancel_all_timer(), which is called
from rtw_usb_if1_deinit() when we remove the USB device. So something
more complicated is wrong. I would prefer to just note this as a bug
until we can investigate more completely.
I believe we can del_timer() twice without creating a bug, but I'm not
positive.
regards,
dan carpenter
> *If* then timer_setup() belongs in _rtw_init_recv_priv() then this is
> where the _cancel_timer_ex() belongs, yes. But what about if the devs
> hid it in a different wrong place?
>
> Right the del_timer is in rtw_cancel_all_timer(), which is called
> from rtw_usb_if1_deinit() when we remove the USB device. So something
> more complicated is wrong. I would prefer to just note this as a bug
> until we can investigate more completely.
Sounds fair.
>
> I believe we can del_timer() twice without creating a bug, but I'm not
> positive.
Yes, we can. I tried it yesterday and it didn't create any bug.
On Wed, Mar 9, 2022 at 1:30 PM Dan Carpenter <[email protected]> wrote:
>
> On Wed, Mar 09, 2022 at 02:25:09AM +0530, Vihas Makwana wrote:
> > The _rtw_init_recv_priv() initializes precvpriv->signal_stat_timer and
> > sets it's timeout interval to 1000 ms. But _rtw_free_recv_priv()
> > doesn't cancel the timer and we need to explicitly call
> > _cancel_timer_ex() after we call _rtw_free_recv_priv() to cancel the
> > timer.
> > Call _cancel_timer_ex() from inside _rtw_free_recv_priv() as every init
> > function needs a matching free function.
> >
> >
> > Signed-off-by: Vihas Makwana <[email protected]>
> > ---
> > drivers/staging/r8188eu/core/rtw_recv.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/r8188eu/core/rtw_recv.c b/drivers/staging/r8188eu/core/rtw_recv.c
> > index d77d98351..61308eb39 100644
> > --- a/drivers/staging/r8188eu/core/rtw_recv.c
> > +++ b/drivers/staging/r8188eu/core/rtw_recv.c
> > @@ -103,6 +103,7 @@ void _rtw_free_recv_priv(struct recv_priv *precvpriv)
> > vfree(precvpriv->pallocated_frame_buf);
> >
> > rtl8188eu_free_recv_priv(padapter);
> > + _cancel_timer_ex(&precvpriv->signal_stat_timer);
> > }
>
> *If* then timer_setup() belongs in _rtw_init_recv_priv() then this is
> where the _cancel_timer_ex() belongs, yes. But what about if the devs
> hid it in a different wrong place?
>
> Right the del_timer is in rtw_cancel_all_timer(), which is called
> from rtw_usb_if1_deinit() when we remove the USB device. So something
> more complicated is wrong. I would prefer to just note this as a bug
> until we can investigate more completely.
>
> I believe we can del_timer() twice without creating a bug, but I'm not
> positive.
>
> regards,
> dan carpenter
>
--
Thanks,
Vihas