Re: [PATCH 3/3] xfs: Fix stale data exposure when readahead races with hole punch

[Boaz Harrosh <openosd@gmail.com>]

On 24/09/2019 18:23, Jan Kara wrote:
> On Mon 23-09-19 15:33:05, Boaz Harrosh wrote:
>> On 18/09/2019 15:31, Jan Kara wrote:
>> <>
>>>>> Is there a test on xfstests to demonstrate this race?
>>>>
>>>> No, but I can try to create one.
>>>
>>> I was experimenting with this but I could not reproduce the issue in my
>>> test VM without inserting artificial delay at appropriate place... So I
>>> don't think there's much point in the fstest for this.
>>>
>>> 								Honza
>>>
>>
>> If I understand correctly you will need threads that direct-write
>> files, then fadvise(WILL_NEED) - in parallel to truncate (punch_hole) these
>> files - In parallel to trash caches.
>> (Direct-write is so data is not present in cache when you come to WILL_NEED
>>  it into the cache, otherwise the xfs b-trees are not exercised. Or are you
>>  more worried about the page_cache races?
>> )
> 
> What I was testing was:
>   Fill file with data.

But are you sure data is not in page cache after this stage?

Also this stage sould create multiple extents perhaps with gaps in between

>   One process does fadvise(WILLNEED) block by block from end of the file.
>   Another process punches hole into the file.
> 

(Perhaps randome placement that spans multiple extents in one go)

> If they race is the right way, following read will show old data instead of
> zeros. And as I said I'm able to hit this but only if I add artificial
> delay between truncating page cache and actually removing blocks.
> 

I was more afraid of iterating on a btree or xarray in parallel of it being
destroyed / punched.

I think if you iterate backwards in the WILLNEED case the tree/xarry corruption is
less likely

But now that I think about it. Maybe your case is very different to mine, because
read_pages() in xfs does take the ilock. I'm not familiar with this code.

> 								Honza
> 

But I guess the window is very small then

Thanks
Boaz