Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] staging: rtl8723bs: Use list iterators and helpers
To:     Dan Carpenter <dan.carpenter@oracle.com>,
        Fabio Aiuto <fabioaiuto83@gmail.com>
Cc:     Guenter Roeck <linux@roeck-us.net>, gregkh@linuxfoundation.org,
        Larry.Finger@lwfinger.net, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
References: <20210428173301.149619-1-linux@roeck-us.net>
 <20210604172632.GA1526@agape.jhs> <20210605005422.GB255680@roeck-us.net>
 <20210607111704.GT1955@kadam> <20210607112648.GA314533@roeck-us.net>
 <20210607113624.GA1410@agape.jhs> <20210607121106.GU1955@kadam>
From:   Hans de Goede <hdegoede@redhat.com>
Message-ID: <a2fc4f6d-a7c7-b8ca-3633-10479a183f20@redhat.com>
Date:   Mon, 7 Jun 2021 17:34:51 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <20210607121106.GU1955@kadam>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 6/7/21 2:11 PM, Dan Carpenter wrote:
> On Mon, Jun 07, 2021 at 01:36:24PM +0200, Fabio Aiuto wrote:
>> 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.
> 
> Thanks Fabio.  I feel like we can fix this.  No need to revert.

Ack. My suggestion for reverting this was because I did not know a fix was
in the works / almost ready, fixing it indeed is better,

Regards,

Hans