On Fri, Jul 09, 2021 at 05:36:22PM +0800, Anand Jain wrote:

On 5/7/21 10:01 am, Qu Wenruo wrote:

quoted

Raid5/6 is not only unsafe due to its write-hole problem, but also has
tons of hardcoded PAGE_SIZE.

So disable it for subpage support for now.

Signed-off-by: Qu Wenruo <redacted>
---
  fs/btrfs/disk-io.c | 10 ++++++++++
  fs/btrfs/volumes.c |  8 ++++++++
  2 files changed, 18 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b117dd3b8172..3de8e86f3170 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c

@@ -3402,6 +3402,16 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
  			goto fail_alloc;
  		}
  	}
+	if (sectorsize != PAGE_SIZE) {
+		if (btrfs_super_incompat_flags(fs_info->super_copy) &
+			BTRFS_FEATURE_INCOMPAT_RAID56) {
+			btrfs_err(fs_info,
+	"raid5/6 is not yet supported for sector size %u with page size %lu",
+				sectorsize, PAGE_SIZE);
+			err = -EINVAL;
+			goto fail_alloc;
+		}
+	}
  
  	ret = btrfs_init_workqueues(fs_info, fs_devices);
  	if (ret) {

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 782e16795bc4..3cd876c49446 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c

@@ -3937,11 +3937,19 @@ static inline int validate_convert_profile(struct btrfs_fs_info *fs_info,
  	if (!(bargs->flags & BTRFS_BALANCE_ARGS_CONVERT))
  		return true;
  
+	if (fs_info->sectorsize < PAGE_SIZE &&
+		bargs->target & BTRFS_BLOCK_GROUP_RAID56_MASK) {
+		btrfs_err(fs_info,
+	"RAID5/6 is not supported yet for sectorsize %u with page size %lu",
+			  fs_info->sectorsize, PAGE_SIZE);
+		goto invalid;

Just
  return false;
is fine.

The above btrfs_err() will suffice. We don't need additional btrfs_err() 
at the label invalid.

Agreed, this is the pattern with several checks and any of them can fail
in place as tehre's no common cleanup at the end of the function.