Re: [PATCH 2/4] btrfs: use sb state to print space_cache mount option

Boris Burkov
2020-09-21T17:20:10Z

On Mon, Sep 21, 2020 at 07:04:05PM +0200, David Sterba wrote:
> On Mon, Sep 21, 2020 at 10:50:25AM -0400, Josef Bacik wrote:
> > On 9/17/20 2:13 PM, Boris Burkov wrote:
> > > To make the contents of /proc/mounts better match the actual state of
> > > the file system, base the display of the space cache mount options off
> > > the contents of the super block rather than the last mount options
> > > passed in. Since there are many scenarios where the mount will ignore a
> > > space cache option, simply showing the passed in option is misleading.
> > >
> > > For example, if we mount with -o remount,space_cache=v2 on a read-write
> > > file system without an existing free space tree, we won't build a free
> > > space tree, but /proc/mounts will read space_cache=v2 (until we mount
> > > again and it goes away)
> > >
> > > There is already mount logic based on the super block's cache_generation
> > > and free space tree flag that helps decide a consistent setting for the
> > > space cache options, so we just bring those further to the fore. For
> > > free space tree, the flag is already consistent, so we just switch mount
> > > option display to use it. cache_generation is not always reliably set
> > > correctly, so we ensure that cache_generation > 0 iff the file system
> > > is using space_cache v1. This requires committing a transaction on any
> > > mount which changes whether we are using v1. (v1->nospace_cache, v1->v2,
> > > nospace_cache->v1, v2->v1).
> > >
> > > References: https://github.com/btrfs/btrfs-todo/issues/5
> > > Signed-off-by: Boris Burkov <boris@bur.io>
> >
> > Dave already took this, but next time I'd prefer if we'd keep logical changes
> > separate. So one patch to change /proc/mounts, one patch to deal with clearing
> > the free space generation field if we're not using it.
>
> I haven't taken it yet, adding branches to for-next is only to get test
> coverage, by 'taken' you can count addig it to misc-next.

Would you guys like me to split it up? I felt it was worth keeping the
two changes together because changing the mount options without clearing
the generation will break /proc/mounts in cases that currently work OK.

e.g., I believe it will never show space_cache=v2 if you have the
space_cache if look at generation, and the generation has ever been set.