Talk about why someone should call this function instead of what this function does. This makes the long comment in mm/readahead.c obsolete; replace it with a comment about which case we're protecting against. Signed-off-by: Matthew Wilcox (Oracle) --- include/linux/sched/mm.h | 31 +++++++++++++++++++++++-------- mm/readahead.c | 14 ++------------ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h index d7c6a942aa7e..f76be1141b06 100644 --- a/include/linux/sched/mm.h +++ b/include/linux/sched/mm.h @@ -372,13 +372,26 @@ static inline void memalloc_noio_restore(unsigned int flags) } /** - * memalloc_nofs_save - Marks implicit GFP_NOFS allocation scope. + * memalloc_nofs_save - Prevent recursion into the filesystem. * - * This functions marks the beginning of the GFP_NOFS allocation scope. - * All further allocations will implicitly drop __GFP_FS flag and so - * they are safe for the FS critical section from the allocation recursion - * point of view. Use memalloc_nofs_restore to end the scope with flags - * returned by this function. + * All memory allocations between calling this function and calling + * memalloc_nofs_restore() will be prevented from calling into filesystems + * to reclaim memory. Clean page cache memory can still be reclaimed, + * but (for example) inodes will not be. + * + * The primary reason to do this is that the caller has taken a lock + * which would be needed by FS reclaim. While we could theoretically + * call into a different filesystem in this case, it can be a deep call + * stack so it is better to avoid all filesystems. + * + * Filesystems often choose to incorporate a call to this function as part + * of starting a journal transaction. While not a lock in the normal + * sense, it has much the same effect as nested journal transactions + * are either prohibited or expensive. + * + * Also call this function if you need to allocate memory while holding + * a file folio locked. High order allocations (such as those requested + * by slab) can trigger compaction which will attempt to lock the folio. * * Context: This function is safe to be used from any context. * Return: The saved flags to be passed to memalloc_nofs_restore. @@ -389,10 +402,12 @@ static inline unsigned int memalloc_nofs_save(void) } /** - * memalloc_nofs_restore - Ends the implicit GFP_NOFS scope. + * memalloc_nofs_restore - End filesystem reclaim scope. * @flags: Flags to restore. * - * Ends the implicit GFP_NOFS scope started by memalloc_nofs_save function. + * Ends the implicit memory allocation scope started by + * memalloc_nofs_save(). This may not enable access to filesystem reclaim + * if it was already disabled at the time memalloc_nofs_save() was called. * Always make sure that the given flags is the return value from the * pairing memalloc_nofs_save call. */ diff --git a/mm/readahead.c b/mm/readahead.c index 6e5563290287..9c116d4ba963 100644 --- a/mm/readahead.c +++ b/mm/readahead.c @@ -227,17 +227,7 @@ void page_cache_ra_unbounded(struct readahead_control *ractl, gfp_t gfp_mask = readahead_gfp_mask(mapping); unsigned long mark = ULONG_MAX, i = 0; unsigned int min_nrpages = mapping_min_folio_nrpages(mapping); - - /* - * Partway through the readahead operation, we will have added - * locked pages to the page cache, but will not yet have submitted - * them for I/O. Adding another page may need to allocate memory, - * which can trigger memory reclaim. Telling the VM we're in - * the middle of a filesystem operation will cause it to not - * touch file-backed pages, preventing a deadlock. Most (all?) - * filesystems already specify __GFP_NOFS in their mapping's - * gfp_mask, but let's be explicit here. - */ + /* Allocating with locked folios */ unsigned int nofs = memalloc_nofs_save(); lockdep_assert_held(&mapping->invalidate_lock); @@ -512,7 +502,7 @@ void page_cache_ra_order(struct readahead_control *ractl, ra->order = new_order; - /* See comment in page_cache_ra_unbounded() */ + /* Allocating with locked folios */ nofs = memalloc_nofs_save(); filemap_invalidate_lock_shared(mapping); /* -- 2.47.3