On Mon, Jun 16, 2025 at 02:59:24PM +0530, Praneesh P wrote: > On 6/5/2025 4:19 PM, Baochen Qiang wrote: > > On 6/5/2025 6:00 PM, Johan Hovold wrote: > >> On Thu, Jun 05, 2025 at 04:41:32PM +0800, Baochen Qiang wrote: > >>> On 6/4/2025 10:45 PM, Johan Hovold wrote: > >>>> Add the missing memory barrier to make sure that destination ring > >>>> descriptors are read after the head pointers to avoid using stale data > >>>> on weakly ordered architectures like aarch64. > >>>> > >>>> The barrier is added to the ath12k_hal_srng_access_begin() helper for > >>>> symmetry with follow-on fixes for source ring buffer corruption which > >>>> will add barriers to ath12k_hal_srng_access_end(). > >>>> > >>>> Note that this may fix the empty descriptor issue recently worked around > >>>> by commit 51ad34a47e9f ("wifi: ath12k: Add drop descriptor handling for > >>>> monitor ring"). > >>> why? I would expect drunk cookies are valid in case of HAL_MON_DEST_INFO0_EMPTY_DESC, > >>> rather than anything caused by reordering. > >> Based on a quick look it seemed like this could possibly fall in the > >> same category as some of the other workarounds I've spotted while > >> looking into these ordering issues (e.g. f9fff67d2d7c ("wifi: ath11k: > >> Fix SKB corruption in REO destination ring")). > >> > >> If you say this one is clearly unrelated, I'll drop the comment. > > Praneesh, could you comment here since you made that change? > Empty/Drop descriptor is intentionally issued by the hardware during > backpressure scenario > and is unrelated to the issue discussed in this patch series. Thanks for confirming. I've dropped this comment in v3: https://lore.kernel.org/lkml/20250617084402.14475-1-johan+linaro@xxxxxxxxxx/ Johan