Re: Do btrfs compression option changes need to be atomic?

[David Howells <dhowells@redhat.com>]

David Sterba <dsterba@suse.cz> wrote:

> Do you mean the compression type (btrfs_fs_info::compress_type) and the
> related bits in btrfs_fs_info::mount_opt ? There are more compression
> types but not used in the mount context.  I assume you're interested
> only in the mount-time settings, otherwise the defrag and per-inode
> compression has higher priority over the global settings.

Yes.  However, it would appear that remount can race with using the fs_info
variables, so the settings can be changing whilst you're going through the
compress_file_range() function.

I'm not sure it'll hurt exactly, but you can also find, say, DATACOW being
disabled whilst you're considering compressing.

> > Further to that, how much of an issue is it if the configuration is split
> > out into its own struct that is accessed from struct btrfs_fs_info using
> > RCU?
> 
> Depends on how intrusive it's going to be, the mount opions are tested
> at many places. The RCU overhead and "locking" is lightweight enough so
> it should not be a problem in principle, but without seeing the code I
> can't tell.

Actually, a better way of doing this might be to put a subset of the settings
into a single variable, say:

	struct btrfs_fs_info {
		...
		unsigned int	data_storage_opt;
	#define BTRFS_DATA_NONE			0x0000
	#define BTRFS_DATA_COW			0x0001
	#define BTRFS_DATA_SUM			0x0002
	#define BTRFS_DATA_COMPRESS		0x0003
	#define BTRFS_DATA_FORCE_COMPRESS	0x0004
	#define BTRFS_DATA__STORAGE_OPT		0x000f
	#define BTRFS_DATA_COMPRESS_ZLIB	0x0000
	#define BTRFS_DATA_COMPRESS_LZO		0x0010
	#define BTRFS_DATA_COMPRESS_ZSTD	0x0020
	#define BTRFS_DATA__COMPRESS_TYPE	0x00f0
	#define BTRFS_DATA__COMPRESS_LEVEL	0x0f00
		...
	};

That way it might be possible for the datacow, datasum, compress and
compress-force options to be handled atomically.

David