On Tue 21-06-22 10:49:48, Amir Goldstein wrote: > > How exactly do you imagine the synchronization of buffered read against > > buffered write would work? Lock all pages for the read range in the page > > cache? You'd need to be careful to not bring the machine OOM when someone > > asks to read a huge range... > > I imagine that the atomic r/w synchronisation will remain *exactly* as it is > today by taking XFS_IOLOCK_SHARED around generic_file_read_iter(), > when reading data into user buffer, but before that, I would like to issue > and wait for read of the pages in the range to reduce the probability > of doing the read I/O under XFS_IOLOCK_SHARED. > > The pre-warm of page cache does not need to abide to the atomic read > semantics and it is also tolerable if some pages are evicted in between > pre-warn and read to user buffer - in the worst case this will result in > I/O amplification, but for the common case, it will be a big win for the > mixed random r/w performance on xfs. > > To reduce risk of page cache thrashing we can limit this optimization > to a maximum number of page cache pre-warm. > > The questions are: > 1. Does this plan sound reasonable? Ah, I see now. So essentially the idea is to pull the readahead (which is currently happening from filemap_read() -> filemap_get_pages()) out from under the i_rwsem. It looks like a fine idea to me. > 2. Is there a ready helper (force_page_cache_readahead?) that > I can use which takes the required page/invalidate locks? page_cache_sync_readahead() should be the function you need. It does take care to lock invalidate_lock internally when creating & reading pages. I just cannot comment on whether calling this without i_rwsem does not break some internal XFS expectations for stuff like reflink etc. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR