On 12/10/19 9:55 AM, Matthew Wilcox wrote: > On Tue, Dec 10, 2019 at 09:24:52AM -0700, Jens Axboe wrote: >> +/* >> + * Start writeback on the pages in pgs[], and then try and remove those pages >> + * from the page cached. Used with RWF_UNCACHED. >> + */ >> +void write_drop_cached_pages(struct page **pgs, struct address_space *mapping, >> + unsigned *nr) > > It would seem more natural to use a pagevec instead of pgs/nr. I did look into that, but they are intertwined with LRU etc. I deliberately avoided the LRU on the read side, as it adds noticeable overhead and gains us nothing since the pages will be dropped agian. >> +{ >> + loff_t start, end; >> + int i; >> + >> + end = 0; >> + start = LLONG_MAX; >> + for (i = 0; i < *nr; i++) { >> + struct page *page = pgs[i]; >> + loff_t off; >> + >> + off = (loff_t) page_to_index(page) << PAGE_SHIFT; > > Isn't that page_offset()? I guess it is! I'll make that change. >> + __filemap_fdatawrite_range(mapping, start, end, WB_SYNC_NONE); >> + >> + for (i = 0; i < *nr; i++) { >> + struct page *page = pgs[i]; >> + >> + lock_page(page); >> + if (page->mapping == mapping) { > > So you're protecting against the page being freed and reallocated to a > different file, but not against the page being freed and reallocated > to a location in the same file which is outside (start, end)? I guess so, we can add that too, probably just check if the index is still the same. More of a behavioral thing, shouldn't be any correctness issues there. -- Jens Axboe