>-----Original Message-----
>From: Jeff Johnson (QUIC) <quic_jjohnson@xxxxxxxxxxx>
>Sent: Thursday, October 13, 2022 12:20 PM
>To: Sriram R (QUIC) <quic_srirrama@xxxxxxxxxxx>; Kalle Valo
><kvalo@xxxxxxxxxx>; linux-wireless@xxxxxxxxxxxxxxx
>Cc: ath12k@xxxxxxxxxxxxxxxxxxx
>Subject: Re: [PATCH 15/50] wifi: ath12k: add dp_rx.c
>
>On 10/12/2022 4:39 PM, Sriram R (QUIC) wrote:
>>>> + elem = kzalloc(sizeof(*elem), GFP_ATOMIC);
>>>> + if (!elem)
>>>> + goto free_desc;
>>>> +
>>>> + elem->ts = jiffies;
>>>> + memcpy(&elem->data, rx_tid, sizeof(*rx_tid));
>>>> +
>>>> + spin_lock_bh(&dp->reo_cmd_lock);
>>>> + list_add_tail(&elem->list, &dp->reo_cmd_cache_flush_list);
>>>> + dp->reo_cmd_cache_flush_count++;
>>>> +
>>>> + /* Flush and invalidate aged REO desc from HW cache */
>>>> + list_for_each_entry_safe(elem, tmp, &dp-
>>reo_cmd_cache_flush_list,
>>>> + list) {
>>>> + if (dp->reo_cmd_cache_flush_count >
>>> ATH12K_DP_RX_REO_DESC_FREE_THRES ||
>>>> + time_after(jiffies, elem->ts +
>>>> +
>>> msecs_to_jiffies(ATH12K_DP_RX_REO_DESC_FREE_TIMEOUT_MS))) {
>>>> + list_del(&elem->list);
>>>> + dp->reo_cmd_cache_flush_count--;
>>>> + spin_unlock_bh(&dp->reo_cmd_lock);
>>>> +
>>>> + ath12k_dp_reo_cache_flush(ab, &elem->data);
>>>> + kfree(elem);
>>>> + spin_lock_bh(&dp->reo_cmd_lock);
>>>
>>> is this really a safe iteration if you unlock & lock in the middle?
>>> what prevents the tmp node from being deleted during this window?
>> The reo_cmd_cache_flush_list is used in only two contexts, one is this
>> Function called from napi and the other in ath12k_dp_free during core
>> destroy. Before dp_free, the irqs would be disabled and would wait
>> synchronize. Hence there wouldn’t be any race against add or delete to
>> this list. Please let me know if that’s fine.
>
>please add that as a code comment since unlock/do something/lock is a
>"code smell" so you should justify the smell
Sure Jeff.
Thanks,
Sriram.R
