Re: [PATCH] staging: rtl8723bs: Use list iterators and helpers

[Fabio Aiuto <fabioaiuto83@gmail.com>]

On Mon, Jun 07, 2021 at 04:26:48AM -0700, Guenter Roeck wrote:
> On Mon, Jun 07, 2021 at 02:17:04PM +0300, Dan Carpenter wrote:
> > On Fri, Jun 04, 2021 at 05:54:22PM -0700, Guenter Roeck wrote:
> > > On Fri, Jun 04, 2021 at 07:26:33PM +0200, Fabio Aiuto wrote:
> > > > Hello Guenter,
> > > > 
> > > > On Wed, Apr 28, 2021 at 10:33:01AM -0700, Guenter Roeck wrote:
> > > > > The rtl8723bs driver manually re-implements list helper functions
> > > > > and macros in various ways. Replace with existing list helpers.
> > > > 
> > > > I'm testing rtl8723bs on a baytrail tablet (Lenovo Ideapad MIIX 300-10IBY)
> > > > and applying the tag staging-5.13-rc4, loading r8723bs makes the whole
> > > > system freezing while trying to connect to local AP.
> > > > 
> > > > Only a power off is allowed.
> > > > 
> > > > I found that commit b3cd518c5abd42fbc747ef55a5fdc40bf7bf01c0
> > > > (staging: rtl8723bs: Use list iterators and helpers)
> > > > introduced the bug.
> > > > 
> > > > I'm trying to find out what's wrong with this patch, have you any suggestions?
> > > 
> > > Some of the iterators needed the _safe variant which I didn't take into account.
> > > I thought that was fixed, but maybe some locations were missed.
> > 
> > Ah...  Crud.  As near as I can tell Martin fixed a lot of these in the driver
> > he is working on, rtl8188eu.  But they still aren't fixed in rtl8723bs.  I looked
> > at the functions and the following loops need to changed to list_for_each_safe().
> > (Doing a search for list_del_init() helped during review).
> > 
> 
> Hans wants the patch introducing the problem reverted, so I won't bother
> sending a fix. Sorry for the trouble. Learned a lesson (I hope).
> 
> Guenter
> 
> > expire_timeout_chk() (both loops)
> > rtw_acl_remove_sta()
> > rtw_sta_flush()
> > stop_ap_mode()
> > 
> > rtw_free_network_queue()
> > chk_bmc_sleepq_hdl()
> > rtw_free_all_stainfo()
> > rtw_free_xmitframe_queue()
> > dequeue_xmitframes_to_sleeping_queue()
> > wakeup_sta_to_xmit()  (both loops)
> > xmit_delivery_enabled_frames()
> > 
> > xmit_xmitframes()
> > cfg80211_rtw_del_station()
> > 
> > regards,
> > dan carpenter
> > 
> > 

thanks to Guenter suggestion I made the fix needed, if there's no particular
hurry to revert the change I'm submitting it today or tomorrow at most.
Will cross check with Dan's hint either.

thank you,

fabio