diff mbox series

[16/17] btrfs: zoned: finish fully written block group

Message ID 59c069e3890f3cbc7fa425cdcf756d241a8bfc92.1628690222.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series ZNS Support for Btrfs | expand

Commit Message

Naohiro Aota Aug. 11, 2021, 2:16 p.m. UTC 
If we have written to the zone capacity, the device automatically
deactivates the zone. Sync up block group side (the active BG list and
zone_is_active flag) with it.

We need to do it both on data BGs and metadata BGs. On data side, we add a
hook to btrfs_finish_ordered_io(). On metadata side, we use
end_extent_buffer_writeback().

To reduce excess lookup of a block group, we mark the last extent buffer in
a block group with EXTENT_BUFFER_ZONE_FINISH flag. This cannot be done for
data (ordered_extent), because the address may change due to
REQ_OP_ZONE_APPEND.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/extent_io.c | 11 ++++++++-
 fs/btrfs/extent_io.h |  1 +
 fs/btrfs/inode.c     |  6 ++++-
 fs/btrfs/zoned.c     | 58 ++++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/zoned.h     |  8 ++++++
 5 files changed, 82 insertions(+), 2 deletions(-)

Comments

kernel test robot Aug. 11, 2021, 5:26 p.m. UTC | #1 
Hi Naohiro,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[cannot apply to v5.14-rc5 next-20210811]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: hexagon-randconfig-r041-20210810 (attached as .config)
compiler: clang version 12.0.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ccecd271dc2436fe587af8d2083c3c96ab86e309
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
        git checkout ccecd271dc2436fe587af8d2083c3c96ab86e309
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=hexagon 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/zoned.c:1948:2: warning: variable 'ret' is used uninitialized whenever '?:' condition is true [-Wsometimes-uninitialized]
           ASSERT(!list_empty(&block_group->active_bg_list));
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/ctree.h:3458:3: note: expanded from macro 'ASSERT'
           (likely(expr) ? (void)0 : assertfail(#expr, __FILE__, __LINE__))
            ^~~~~~~~~~~~
   include/linux/compiler.h:77:20: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/zoned.c:1956:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/zoned.c:1948:2: note: remove the '?:' if its condition is always false
           ASSERT(!list_empty(&block_group->active_bg_list));
           ^
   fs/btrfs/ctree.h:3458:3: note: expanded from macro 'ASSERT'
           (likely(expr) ? (void)0 : assertfail(#expr, __FILE__, __LINE__))
            ^
   include/linux/compiler.h:77:20: note: expanded from macro 'likely'
   # define likely(x)      __builtin_expect(!!(x), 1)
                           ^
   fs/btrfs/zoned.c:1908:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +1948 fs/btrfs/zoned.c

  1900	
  1901	int btrfs_zone_finish_endio(struct btrfs_fs_info *fs_info, u64 logical,
  1902				    u64 length)
  1903	{
  1904		struct btrfs_block_group *block_group;
  1905		struct map_lookup *map;
  1906		struct btrfs_device *device;
  1907		u64 physical;
  1908		int ret;
  1909	
  1910		if (!btrfs_is_zoned(fs_info))
  1911			return 0;
  1912	
  1913		block_group = btrfs_lookup_block_group(fs_info, logical);
  1914		ASSERT(block_group);
  1915	
  1916		if (logical + length < block_group->start + block_group->zone_capacity) {
  1917			ret = 0;
  1918			goto out;
  1919		}
  1920	
  1921		spin_lock(&block_group->lock);
  1922	
  1923		if (!block_group->zone_is_active) {
  1924			spin_unlock(&block_group->lock);
  1925			ret = 0;
  1926			goto out;
  1927		}
  1928	
  1929		block_group->zone_is_active = 0;
  1930		/* We should have consumed all the free space */
  1931		ASSERT(block_group->alloc_offset == block_group->zone_capacity);
  1932		ASSERT(block_group->free_space_ctl->free_space == 0);
  1933		btrfs_clear_treelog_bg(block_group);
  1934		spin_unlock(&block_group->lock);
  1935	
  1936		map = block_group->physical_map;
  1937		device = map->stripes[0].dev;
  1938		physical = map->stripes[0].physical;
  1939	
  1940		if (!device->zone_info->max_active_zones) {
  1941			ret = 0;
  1942			goto out;
  1943		}
  1944	
  1945		btrfs_dev_clear_active_zone(device, physical);
  1946	
  1947		spin_lock(&fs_info->zone_active_bgs_lock);
> 1948		ASSERT(!list_empty(&block_group->active_bg_list));

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Aug. 11, 2021, 6:37 p.m. UTC | #2 
Hi Naohiro,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kdave/for-next]
[cannot apply to v5.14-rc5 next-20210811]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: x86_64-randconfig-a012-20210810 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d39ebdae674c8efc84ebe8dc32716ec353220530)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ccecd271dc2436fe587af8d2083c3c96ab86e309
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
        git checkout ccecd271dc2436fe587af8d2083c3c96ab86e309
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/btrfs/zoned.c:1940:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
           if (!device->zone_info->max_active_zones) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/zoned.c:1956:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/btrfs/zoned.c:1940:2: note: remove the 'if' if its condition is always true
           if (!device->zone_info->max_active_zones) {
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/zoned.c:1908:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +1940 fs/btrfs/zoned.c

  1900	
  1901	int btrfs_zone_finish_endio(struct btrfs_fs_info *fs_info, u64 logical,
  1902				    u64 length)
  1903	{
  1904		struct btrfs_block_group *block_group;
  1905		struct map_lookup *map;
  1906		struct btrfs_device *device;
  1907		u64 physical;
  1908		int ret;
  1909	
  1910		if (!btrfs_is_zoned(fs_info))
  1911			return 0;
  1912	
  1913		block_group = btrfs_lookup_block_group(fs_info, logical);
  1914		ASSERT(block_group);
  1915	
  1916		if (logical + length < block_group->start + block_group->zone_capacity) {
  1917			ret = 0;
  1918			goto out;
  1919		}
  1920	
  1921		spin_lock(&block_group->lock);
  1922	
  1923		if (!block_group->zone_is_active) {
  1924			spin_unlock(&block_group->lock);
  1925			ret = 0;
  1926			goto out;
  1927		}
  1928	
  1929		block_group->zone_is_active = 0;
  1930		/* We should have consumed all the free space */
  1931		ASSERT(block_group->alloc_offset == block_group->zone_capacity);
  1932		ASSERT(block_group->free_space_ctl->free_space == 0);
  1933		btrfs_clear_treelog_bg(block_group);
  1934		spin_unlock(&block_group->lock);
  1935	
  1936		map = block_group->physical_map;
  1937		device = map->stripes[0].dev;
  1938		physical = map->stripes[0].physical;
  1939	
> 1940		if (!device->zone_info->max_active_zones) {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Naohiro Aota Aug. 16, 2021, 4:34 a.m. UTC | #3 
On Thu, Aug 12, 2021 at 01:26:02AM +0800, kernel test robot wrote:
> Hi Naohiro,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on kdave/for-next]
> [cannot apply to v5.14-rc5 next-20210811]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
> config: hexagon-randconfig-r041-20210810 (attached as .config)
> compiler: clang version 12.0.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/0day-ci/linux/commit/ccecd271dc2436fe587af8d2083c3c96ab86e309
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Naohiro-Aota/ZNS-Support-for-Btrfs/20210811-222302
>         git checkout ccecd271dc2436fe587af8d2083c3c96ab86e309
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=hexagon 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> fs/btrfs/zoned.c:1948:2: warning: variable 'ret' is used uninitialized whenever '?:' condition is true [-Wsometimes-uninitialized]
>            ASSERT(!list_empty(&block_group->active_bg_list));
>            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    fs/btrfs/ctree.h:3458:3: note: expanded from macro 'ASSERT'
>            (likely(expr) ? (void)0 : assertfail(#expr, __FILE__, __LINE__))
>             ^~~~~~~~~~~~
>    include/linux/compiler.h:77:20: note: expanded from macro 'likely'
>    # define likely(x)      __builtin_expect(!!(x), 1)
>                            ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    fs/btrfs/zoned.c:1956:9: note: uninitialized use occurs here
>            return ret;
>                   ^~~
>    fs/btrfs/zoned.c:1948:2: note: remove the '?:' if its condition is always false
>            ASSERT(!list_empty(&block_group->active_bg_list));
>            ^
>    fs/btrfs/ctree.h:3458:3: note: expanded from macro 'ASSERT'
>            (likely(expr) ? (void)0 : assertfail(#expr, __FILE__, __LINE__))
>             ^
>    include/linux/compiler.h:77:20: note: expanded from macro 'likely'
>    # define likely(x)      __builtin_expect(!!(x), 1)
>                            ^
>    fs/btrfs/zoned.c:1908:9: note: initialize the variable 'ret' to silence this warning
>            int ret;
>                   ^
>                    = 0
>    1 warning generated.

This looks false-positive for me but still I noticed this "ret" can
never be non-zero in any path. So, I'll convert the function to "void"
in the next version.
diff mbox series

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index aaddd7225348..c353bfd89dfc 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -4155,6 +4155,9 @@  void wait_on_extent_buffer_writeback(struct extent_buffer *eb)
 
 static void end_extent_buffer_writeback(struct extent_buffer *eb)
 {
+	if (test_bit(EXTENT_BUFFER_ZONE_FINISH, &eb->bflags))
+		btrfs_zone_finish_endio(eb->fs_info, eb->start, eb->len);
+
 	clear_bit(EXTENT_BUFFER_WRITEBACK, &eb->bflags);
 	smp_mb__after_atomic();
 	wake_up_bit(&eb->bflags, EXTENT_BUFFER_WRITEBACK);
@@ -4756,8 +4759,14 @@  static int submit_eb_page(struct page *page, struct writeback_control *wbc,
 		free_extent_buffer(eb);
 		return ret;
 	}
-	if (cache)
+	if (cache) {
+		/* Impiles write in zoned btrfs*/
 		btrfs_put_block_group(cache);
+		/* Mark the last eb in a block group */
+		if (cache->seq_zone &&
+		    eb->start + eb->len == cache->zone_capacity)
+			set_bit(EXTENT_BUFFER_ZONE_FINISH, &eb->bflags);
+	}
 	ret = write_one_eb(eb, wbc, epd);
 	free_extent_buffer(eb);
 	if (ret < 0)
diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
index 53abdc280451..9f3e0a45a5e4 100644
--- a/fs/btrfs/extent_io.h
+++ b/fs/btrfs/extent_io.h
@@ -32,6 +32,7 @@  enum {
 	/* write IO error */
 	EXTENT_BUFFER_WRITE_ERR,
 	EXTENT_BUFFER_NO_CHECK,
+	EXTENT_BUFFER_ZONE_FINISH,
 };
 
 /* these are flags for __process_pages_contig */
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index afe9dcda860b..1697f745ba5c 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3010,8 +3010,12 @@  static int btrfs_finish_ordered_io(struct btrfs_ordered_extent *ordered_extent)
 		goto out;
 	}
 
-	if (ordered_extent->bdev)
+	/* Non-null bdev implies a write on a sequential zone */
+	if (ordered_extent->bdev) {
 		btrfs_rewrite_logical_zoned(ordered_extent);
+		btrfs_zone_finish_endio(fs_info, ordered_extent->disk_bytenr,
+					logical_len);
+	}
 
 	btrfs_free_io_failure_record(inode, start, end);
 
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 850662d103e9..dd92e48b7f56 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1897,3 +1897,61 @@  bool btrfs_can_activate_zone(struct btrfs_fs_devices *fs_devices,
 
 	return ret;
 }
+
+int btrfs_zone_finish_endio(struct btrfs_fs_info *fs_info, u64 logical,
+			    u64 length)
+{
+	struct btrfs_block_group *block_group;
+	struct map_lookup *map;
+	struct btrfs_device *device;
+	u64 physical;
+	int ret;
+
+	if (!btrfs_is_zoned(fs_info))
+		return 0;
+
+	block_group = btrfs_lookup_block_group(fs_info, logical);
+	ASSERT(block_group);
+
+	if (logical + length < block_group->start + block_group->zone_capacity) {
+		ret = 0;
+		goto out;
+	}
+
+	spin_lock(&block_group->lock);
+
+	if (!block_group->zone_is_active) {
+		spin_unlock(&block_group->lock);
+		ret = 0;
+		goto out;
+	}
+
+	block_group->zone_is_active = 0;
+	/* We should have consumed all the free space */
+	ASSERT(block_group->alloc_offset == block_group->zone_capacity);
+	ASSERT(block_group->free_space_ctl->free_space == 0);
+	btrfs_clear_treelog_bg(block_group);
+	spin_unlock(&block_group->lock);
+
+	map = block_group->physical_map;
+	device = map->stripes[0].dev;
+	physical = map->stripes[0].physical;
+
+	if (!device->zone_info->max_active_zones) {
+		ret = 0;
+		goto out;
+	}
+
+	btrfs_dev_clear_active_zone(device, physical);
+
+	spin_lock(&fs_info->zone_active_bgs_lock);
+	ASSERT(!list_empty(&block_group->active_bg_list));
+	list_del_init(&block_group->active_bg_list);
+	spin_unlock(&fs_info->zone_active_bgs_lock);
+
+	btrfs_put_block_group(block_group);
+
+out:
+	btrfs_put_block_group(block_group);
+	return ret;
+}
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index ade6588c4ccd..04a3ea884f3b 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -73,6 +73,8 @@  bool btrfs_zone_activate(struct btrfs_block_group *block_group);
 int btrfs_zone_finish(struct btrfs_block_group *block_group);
 bool btrfs_can_activate_zone(struct btrfs_fs_devices *fs_devices,
 			     int raid_index);
+int btrfs_zone_finish_endio(struct btrfs_fs_info *fs_info, u64 logical,
+			    u64 length);
 #else /* CONFIG_BLK_DEV_ZONED */
 static inline int btrfs_get_dev_zone(struct btrfs_device *device, u64 pos,
 				     struct blk_zone *zone)
@@ -224,6 +226,12 @@  static inline bool btrfs_can_activate_zone(struct btrfs_fs_devices *fs_devices,
 	return true;
 }
 
+static inline int btrfs_zone_finish_endio(struct btrfs_fs_info *fs_info,
+					  u64 logical, u64 length)
+{
+	return 0;
+}
+
 #endif
 
 static inline bool btrfs_dev_is_sequential(struct btrfs_device *device, u64 pos)