Alistair Popple wrote:
> Several functions internal to FS DAX use the following pattern when
> trying to obtain an unlocked entry:
>
> xas_for_each(&xas, entry, end_idx) {
> if (dax_is_locked(entry))
> entry = get_unlocked_entry(&xas, 0);
>
> This is problematic because get_unlocked_entry() will get the next
> present entry in the range, and the next entry may not be
> locked. Therefore any processing of the original locked entry will be
> skipped. This can cause dax_layout_busy_page_range() to miss DMA-busy
> pages in the range, leading file systems to free blocks whilst DMA
> operations are ongoing which can lead to file system corruption.
>
> Instead callers from within a xas_for_each() loop should be waiting
> for the current entry to be unlocked without advancing the XArray
> state so a new function is introduced to wait.
Oh wow, good eye!
Did this trip up an xfstest, or did you see this purely by inspection?
> Also while we are here rename get_unlocked_entry() to
> get_next_unlocked_entry() to make it clear that it may advance the
> iterator state.
Outside of the above clarification of how found / end user effect you
can add:
Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
