Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Fri, 4 Mar 2022 10:04:35 -0800
From:   Jaegeuk Kim <jaegeuk@kernel.org>
To:     Dave Chinner <david@fromorbit.com>
Cc:     linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] vfs: do not try to evict inode when super is frozen
Message-ID: <YiJUsxjoHNiqEaFG@google.com>
References: <20220304022104.2525009-1-jaegeuk@kernel.org>
 <20220304024843.GK3927073@dread.disaster.area>
 <YiGgSmEOZUvgmSto@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YiGgSmEOZUvgmSto@google.com>
Precedence: bulk

On 03/03, Jaegeuk Kim wrote:
> On 03/04, Dave Chinner wrote:
> > On Thu, Mar 03, 2022 at 06:21:04PM -0800, Jaegeuk Kim wrote:
> > > Otherwise, we will get a deadlock.
> > 
> > NACK.
> > 
> > We have to be able to evict clean inodes from memory on frozen
> > inodes because we can still instantiate inodes while the filesytem
> > is frozen. e.g. there's a find running when the filesystem is
> > frozen. What happens if we can't evict clean cached inodes from
> > memory when we run out of memory trying to instantiate new inodes?
> 
> Ok, that makes sense.
> 
> > 
> > > 
> > > [freeze test]                         shrinkder
> > > freeze_super
> > >  - pwercpu_down_write(SB_FREEZE_FS)
> > >                                        - super_cache_scan
> > >                                          - down_read(&sb->s_umount)
> > >                                            - prune_icache_sb
> > >                                             - dispose_list
> > >                                              - evict
> > >                                               - f2fs_evict_inode
> > > thaw_super
> > >  - down_write(&sb->s_umount);
> > >                                               - __percpu_down_read(SB_FREEZE_FS)
> > 
> > That seems like a f2fs bug, not a generic problem.
> > 
> > Filesystems already have to handle stuff like this if an unlinked
> > file is closed while the fs is frozen - we have to handle inode
> > eviction needing to modify the file, and different filesystems
> > handle this differently. Most filesystems simply block in
> > ->evict_inode in this case, but this never occurs from the shrinker
> > context.
> > 
> > IOWs, the shrinker should never be evicting inodes that require the
> > filesystem to immediately block on frozen filesystems. If you have
> > such inodes in cache at the time the filesystem is frozen, then they
> > should be purged from the cache as part of the freeze process so the
> > shrinker won't ever find inodes that it could deadlock on.
> 
> If so, is this a bug in drop_caches_sysctl_handler? Or, I shouldn't have
> used "echo 3 > sysfs/drop_caches" with freezefs in xfstests?

My bad. I totally misunderstood. I'm testing a patch to call evict_inodes()
in f2fs_freeze(). Thank you for the comment. :)

> 
> > 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com