| Message ID | 20210711150927.3898403-1-willy@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Close a hole where IOCB_NOWAIT reads could sleep | expand |
On 7/11/21 9:09 AM, Matthew Wilcox (Oracle) wrote: > I noticed a theoretical case where an IOCB_NOWAIT read could sleep: > > filemap_get_pages > filemap_get_read_batch > page_cache_sync_readahead > page_cache_sync_ra > ondemand_readahead > do_page_cache_ra > page_cache_ra_unbounded > gfp_t gfp_mask = readahead_gfp_mask(mapping); > memalloc_nofs_save() > __page_cache_alloc(gfp_mask); > > We're in a nofs context, so we're not going to start new IO, but we might > wait for writeback to complete. We generally don't want to sleep for IO, > particularly not for IO that isn't related to us. > > Jens, can you run this through your test rig and see if it makes any > practical difference? You bet, I'll see if I can trigger this condition and verify we're no longer blocking on writeback. Thanks for hacking this up.
On Sun, Jul 11, 2021 at 07:44:07PM -0600, Jens Axboe wrote: > On 7/11/21 9:09 AM, Matthew Wilcox (Oracle) wrote: > > I noticed a theoretical case where an IOCB_NOWAIT read could sleep: > > > > filemap_get_pages > > filemap_get_read_batch > > page_cache_sync_readahead > > page_cache_sync_ra > > ondemand_readahead > > do_page_cache_ra > > page_cache_ra_unbounded > > gfp_t gfp_mask = readahead_gfp_mask(mapping); > > memalloc_nofs_save() > > __page_cache_alloc(gfp_mask); > > > > We're in a nofs context, so we're not going to start new IO, but we might > > wait for writeback to complete. We generally don't want to sleep for IO, > > particularly not for IO that isn't related to us. > > > > Jens, can you run this through your test rig and see if it makes any > > practical difference? > > You bet, I'll see if I can trigger this condition and verify we're no > longer blocking on writeback. Thanks for hacking this up. Did you have any success yet?
On 7/24/21 12:22 PM, Matthew Wilcox wrote: > On Sun, Jul 11, 2021 at 07:44:07PM -0600, Jens Axboe wrote: >> On 7/11/21 9:09 AM, Matthew Wilcox (Oracle) wrote: >>> I noticed a theoretical case where an IOCB_NOWAIT read could sleep: >>> >>> filemap_get_pages >>> filemap_get_read_batch >>> page_cache_sync_readahead >>> page_cache_sync_ra >>> ondemand_readahead >>> do_page_cache_ra >>> page_cache_ra_unbounded >>> gfp_t gfp_mask = readahead_gfp_mask(mapping); >>> memalloc_nofs_save() >>> __page_cache_alloc(gfp_mask); >>> >>> We're in a nofs context, so we're not going to start new IO, but we might >>> wait for writeback to complete. We generally don't want to sleep for IO, >>> particularly not for IO that isn't related to us. >>> >>> Jens, can you run this through your test rig and see if it makes any >>> practical difference? >> >> You bet, I'll see if I can trigger this condition and verify we're no >> longer blocking on writeback. Thanks for hacking this up. > > Did you have any success yet? Sorry forgot to report back - I did run some testing last week, and didn't manage to make it hit the blocking condition. Did various read/write mix on the same file, made sure there was memory pressure, etc. I'll give it another go, please let me know if you have an idea on how to make this easier to hit... I know it's one of those things that you hit all the time in certain workloads (hence why I would love to see it get fixed), but I just didn't manage to provoke it when I tried.
I noticed a theoretical case where an IOCB_NOWAIT read could sleep: filemap_get_pages filemap_get_read_batch page_cache_sync_readahead page_cache_sync_ra ondemand_readahead do_page_cache_ra page_cache_ra_unbounded gfp_t gfp_mask = readahead_gfp_mask(mapping); memalloc_nofs_save() __page_cache_alloc(gfp_mask); We're in a nofs context, so we're not going to start new IO, but we might wait for writeback to complete. We generally don't want to sleep for IO, particularly not for IO that isn't related to us. Jens, can you run this through your test rig and see if it makes any practical difference? Matthew Wilcox (Oracle) (2): mm/readahead: Add gfp_flags to ractl mm/filemap: Prevent waiting for memory for NOWAIT reads include/linux/pagemap.h | 3 +++ mm/filemap.c | 31 +++++++++++++++++++------------ mm/readahead.c | 16 ++++++++-------- 3 files changed, 30 insertions(+), 20 deletions(-)