Re: [PATCH] ext4: Fix deadlock on page reclaim

[Dave Chinner <david@fromorbit.com>]

On Thu, Jul 25, 2019 at 04:54:42AM -0700, Christoph Hellwig wrote:
> On Thu, Jul 25, 2019 at 06:33:58PM +0900, Damien Le Moal wrote:
> > +	gfp_t gfp_mask;
> > +
> >  	switch (ext4_inode_journal_mode(inode)) {
> >  	case EXT4_INODE_ORDERED_DATA_MODE:
> >  	case EXT4_INODE_WRITEBACK_DATA_MODE:
> > @@ -4019,6 +4019,14 @@ void ext4_set_aops(struct inode *inode)
> >  		inode->i_mapping->a_ops = &ext4_da_aops;
> >  	else
> >  		inode->i_mapping->a_ops = &ext4_aops;
> > +
> > +	/*
> > +	 * Ensure all page cache allocations are done from GFP_NOFS context to
> > +	 * prevent direct reclaim recursion back into the filesystem and blowing
> > +	 * stacks or deadlocking.
> > +	 */
> > +	gfp_mask = mapping_gfp_mask(inode->i_mapping);
> > +	mapping_set_gfp_mask(inode->i_mapping, (gfp_mask & ~(__GFP_FS)));
> 
> This looks like something that could hit every file systems, so
> shouldn't we fix this in common code?  We could also look into
> just using memalloc_nofs_save for the page cache allocation path
> instead of the per-mapping gfp_mask.

I think it has to be the entire IO path - any allocation from the
underlying filesystem could recurse into the top level filesystem
and then deadlock if the memory reclaim submits IO or blocks on
IO completion from the upper filesystem. That's a bloody big hammer
for something that is only necessary when there are stacked
filesystems like this....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com