On Thu, Jan 16, 2020 at 02:36:01PM +0800, yu kuai wrote: > I noticed that generic/418 test may fail with small probability. And with > futher investiation, it can be reproduced with: > > ./src/dio-invalidate-cache -wp -b 4096 -n 8 -i 1 -f filename > ./src/dio-invalidate-cache -wt -b 4096-n 8 -i 1 -f filename > > The failure is because direct write wrote none-zero but buffer read got > zero. > > In the process of buffer read, if the page do not exist, readahead will > be triggered. __do_page_cache_readahead() will allocate page first. Next, > if the file block is unwritten(or hole), iomap_begin() will set iomap->type > to IOMAP_UNWRITTEN(or IOMAP_HOLE). Then, iomap_readpages_actor() will add > page to page cache. Finally, iomap_readpage_actor() will zero the page. > > However, there is no lock or serialization between initializing iomap and > adding page to page cache against direct write. If direct write happen to > fininsh between them, the type of iomap should be IOMAP_MAPPED instead of > IOMAP_UNWRITTEN or IOMAP_HOLE. And the page will end up zeroed out in page > cache, while on-disk page hold the data of direct write. It's worth noting that my patch series from earlier this week to redesign the readahead API will fix this problem. Direct write will block on the locked pages in the page cache.
