| Message ID | 20200623185032.14983-1-dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: start deprecation of mount option inode_cache | expand |
On 2020/6/24 上午2:50, David Sterba wrote: > Estimated time of removal of the functionality is 5.11, the option will > be still parsed but not doing anything. Great that we don't need to test for a mount option that no one really uses. > > Reasons for deprecation and removal: > > - very poor naming choice of the mount option, it's supposed to cache > and reuse the inode _numbers_, but it sounds a some generic cache for > inodes > > - the only known usecase where this option would make sense is on a > 32bit architecture where inode numbers in one subvolume would be > exhausted due to 32bit inode::i_ino > > - the cache is stored on disk, consumes space, needs to be loaded and > written back > > - new inode number allocation is slower due to lookups into the cache > (compared to a simple increment which is the default) > > - uses the free-space-cache code that is going to be deprecated as well > in the future > > Known problems: > > - since 2011, returning EEXIST when there's not enough space in a page > to store all checksums, see commit 4b9465cb9e38 ("Btrfs: add mount -o > inode_cache") > > Remaining issues: > > - if the option was enabled, new inodes created, the option disabled > again, the cache is still stored on the devices and there's currently > no way to remove it What about "btrfs rescue remove-deprecated-feature inode_cache"? I really don't want kernel to do the hassle. Thanks, Qu > > Signed-off-by: David Sterba <dsterba@suse.com> > --- > fs/btrfs/super.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index 98fe2a634c70..3f1abbeef66c 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -827,6 +827,8 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options, > } > break; > case Opt_inode_cache: > + btrfs_warn(info, > + "the 'inode_cache' option is deprecated and will be stop working in 5.11"); > btrfs_set_pending_and_info(info, INODE_MAP_CACHE, > "enabling inode map caching"); > break; >
On 2020-06-23 19:50, David Sterba wrote: > @@ -827,6 +827,8 @@ int btrfs_parse_options(struct btrfs_fs_info > *info, char *options, > } > break; > case Opt_inode_cache: > + btrfs_warn(info, > + "the 'inode_cache' option is deprecated and will be stop working in > 5.11"); "will stop working", but perhaps "will have no effect from" might be more explicit.
On Wed, Jun 24, 2020 at 08:37:13AM +0100, Steven Davies wrote: > On 2020-06-23 19:50, David Sterba wrote: > > @@ -827,6 +827,8 @@ int btrfs_parse_options(struct btrfs_fs_info > > *info, char *options, > > } > > break; > > case Opt_inode_cache: > > + btrfs_warn(info, > > + "the 'inode_cache' option is deprecated and will be stop working in > > 5.11"); > > "will stop working", but perhaps "will have no effect from" might be > more explicit. Thanks, that's better, I'll update the patch.
On Wed, Jun 24, 2020 at 07:27:46AM +0800, Qu Wenruo wrote: > > Remaining issues: > > > > - if the option was enabled, new inodes created, the option disabled > > again, the cache is still stored on the devices and there's currently > > no way to remove it > > What about "btrfs rescue remove-deprecated-feature inode_cache"? > I really don't want kernel to do the hassle. Most likely we'll need both, the kernel part is for cases where it's not so easy to access the filesystem unmounted to do the change. Like a root partition. How to do that to be least intrusive is the open.
On 2020/6/24 下午7:00, David Sterba wrote: > On Wed, Jun 24, 2020 at 07:27:46AM +0800, Qu Wenruo wrote: >>> Remaining issues: >>> >>> - if the option was enabled, new inodes created, the option disabled >>> again, the cache is still stored on the devices and there's currently >>> no way to remove it >> >> What about "btrfs rescue remove-deprecated-feature inode_cache"? >> I really don't want kernel to do the hassle. > > Most likely we'll need both, the kernel part is for cases where it's not > so easy to access the filesystem unmounted to do the change. Like a root > partition. How to do that to be least intrusive is the open. > OK. Then what about do it in btrfs_orphan_cleanup()? It looks like a perfect match. Thanks, Qu
On Tue, Jun 23, 2020 at 08:50:32PM +0200, David Sterba wrote: > Remaining issues: > > - if the option was enabled, new inodes created, the option disabled > again, the cache is still stored on the devices and there's currently > no way to remove it The diffstat of the removal is b/fs/btrfs/Makefile | 2 b/fs/btrfs/ctree.h | 5 b/fs/btrfs/disk-io.c | 2 b/fs/btrfs/free-space-cache.c | 88 ------ b/fs/btrfs/inode.c | 72 ++++- b/fs/btrfs/ioctl.c | 1 b/fs/btrfs/relocation.c | 1 b/fs/btrfs/super.c | 10 b/fs/btrfs/transaction.c | 16 - b/fs/btrfs/tree-log.c | 2 fs/btrfs/inode-map.c | 580 ------------------------------------------ fs/btrfs/inode-map.h | 16 - 12 files changed, 72 insertions(+), 723 deletions(-) whoever would like to claim it in the 5.11 develoment time will have to provide fix for the remaining issue(s).
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 98fe2a634c70..3f1abbeef66c 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -827,6 +827,8 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options, } break; case Opt_inode_cache: + btrfs_warn(info, + "the 'inode_cache' option is deprecated and will be stop working in 5.11"); btrfs_set_pending_and_info(info, INODE_MAP_CACHE, "enabling inode map caching"); break;
Estimated time of removal of the functionality is 5.11, the option will be still parsed but not doing anything. Reasons for deprecation and removal: - very poor naming choice of the mount option, it's supposed to cache and reuse the inode _numbers_, but it sounds a some generic cache for inodes - the only known usecase where this option would make sense is on a 32bit architecture where inode numbers in one subvolume would be exhausted due to 32bit inode::i_ino - the cache is stored on disk, consumes space, needs to be loaded and written back - new inode number allocation is slower due to lookups into the cache (compared to a simple increment which is the default) - uses the free-space-cache code that is going to be deprecated as well in the future Known problems: - since 2011, returning EEXIST when there's not enough space in a page to store all checksums, see commit 4b9465cb9e38 ("Btrfs: add mount -o inode_cache") Remaining issues: - if the option was enabled, new inodes created, the option disabled again, the cache is still stored on the devices and there's currently no way to remove it Signed-off-by: David Sterba <dsterba@suse.com> --- fs/btrfs/super.c | 2 ++ 1 file changed, 2 insertions(+)