On 2017年09月23日 09:27, Satoru Takeuchi wrote:
At Sat, 23 Sep 2017 10:19:26 +0900,
Satoru Takeuchi wrote:

At Wed, 20 Sep 2017 15:18:43 +0900,
Qu Wenruo wrote:

Commit 7dfb8be11b5d ("btrfs: Round down values which are written for
total_bytes_size") is fixing the unaligned device size caused by
adding/shrinking device.

It added a new WARN_ON() when device size is unaligned.
This is fine for new device added to btrfs using v4.13 kernel, but not
existing device whose total_bytes is already unaligned.

And the WARN_ON() will get triggered every time a block group get
created/removed on the unaligned device.

This patch will remove the WARN_ON(), and warn user more gently what's
happening and how to fix it.

Reported-by: Rich Rauenzahn <r...@shroop.net>
Fixes: 7dfb8be11b5d ("btrfs: Round down values which are written for
total_bytes_size")
Signed-off-by: Qu Wenruo <quwenruo.bt...@gmx.com>
---
  fs/btrfs/ctree.h   |  1 -
  fs/btrfs/volumes.c | 16 ++++++++++++----
  2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5a8933da39a7..4de9269e435a 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1562,7 +1562,6 @@ static inline void btrfs_set_device_total_bytes(struct 
extent_buffer *eb,
  {
        BUILD_BUG_ON(sizeof(u64) !=
                     sizeof(((struct btrfs_dev_item *)0))->total_bytes);
-       WARN_ON(!IS_ALIGNED(val, eb->fs_info->sectorsize));
        btrfs_set_64(eb, s, offsetof(struct btrfs_dev_item, total_bytes), val);
  }
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0e8f16c305df..afae25df6a8c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6472,15 +6472,23 @@ static int read_one_chunk(struct btrfs_fs_info 
*fs_info, struct btrfs_key *key,
        return 0;
  }
-static void fill_device_from_item(struct extent_buffer *leaf,
-                                struct btrfs_dev_item *dev_item,
-                                struct btrfs_device *device)
+static void fill_device_from_item(struct btrfs_fs_info *fs_info,
+                                 struct extent_buffer *leaf,
+                                 struct btrfs_dev_item *dev_item,
+                                 struct btrfs_device *device)
  {
        unsigned long ptr;
device->devid = btrfs_device_id(leaf, dev_item);
        device->disk_total_bytes = btrfs_device_total_bytes(leaf, dev_item);
        device->total_bytes = device->disk_total_bytes;
+       if (!IS_ALIGNED(device->total_bytes, fs_info->sectorsize)) {
+               btrfs_warn(fs_info,
+                          "devid %llu has unaligned total bytes %llu",
+                          device->devid, device->disk_total_bytes);
+               btrfs_warn(fs_info,
+                          "please shrink the device a little and resize back to fix 
it");
+       }

How about telling uses to know device->total_bytes should be alligned
to fs_info->sectorsize here?

Thanks,

I should make my comment clearer, sorry.

===
+       if (!IS_ALIGNED(device->total_bytes, fs_info->sectorsize)) {
+               btrfs_warn(fs_info,
+                          "devid %llu: total bytes %llu should be aligned to %u 
bytes",
+                          device->devid, device->disk_total_bytes, 
fs_info->sectorsize);
+               btrfs_warn(fs_info,
+                          "please shrink the device a little and resize back to fix 
it");
+       }
===

That's better.

But I'm also considering modifying the total_bytes directly here.
So that any time DEV_ITEM and super block get updated, new aligned value will be written back to disk, and since the value is aligned in memory, it won't cause WARN_ON() any longer.

I'll test and check the code for confirmation before updating the patch.

Thanks,
Qu


Thanks,
Satoru

Satoru

        device->commit_total_bytes = device->disk_total_bytes;
        device->bytes_used = btrfs_device_bytes_used(leaf, dev_item);
        device->commit_bytes_used = device->bytes_used;
@@ -6625,7 +6633,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
                        return -EINVAL;
        }
- fill_device_from_item(leaf, dev_item, device);
+       fill_device_from_item(fs_info, leaf, dev_item, device);
        device->in_fs_metadata = 1;
        if (device->writeable && !device->is_tgtdev_for_dev_replace) {
                device->fs_devices->total_rw_bytes += device->total_bytes;
--
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to