[PATCH 15/50] wifi: ath12k: add dp_rx.c

[Jeff Johnson]

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