All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
To: Josef Bacik <josef@toxicpanda.com>,
	Naohiro Aota <Naohiro.Aota@wdc.com>,
	"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	"dsterba@suse.com" <dsterba@suse.com>
Cc: "hare@suse.com" <hare@suse.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v9 19/41] btrfs: redirty released extent buffers in ZONED mode
Date: Fri, 6 Nov 2020 09:11:21 +0000	[thread overview]
Message-ID: <SN4PR0401MB35981D8500BE7FAF931F1AEE9BED0@SN4PR0401MB3598.namprd04.prod.outlook.com> (raw)
In-Reply-To: 6d61ed1d-1801-5710-beac-03d363871ec8@toxicpanda.com

On 03/11/2020 15:43, Josef Bacik wrote:
> This is a lot of work when you could just add
> 
> if (btrfs_is_zoned(fs_info))
> 	return;
> 
> to btrfs_clean_tree_block().  The dirty secret is we don't actually unset the 
> bits in the transaction io tree because it would require memory allocation 
> sometimes, so you don't even need to mess with ->dirty_pages in the first place. 
>   The only thing you need is to keep from clearing the EB dirty.  In fact you 
> could just do
> 
> if (btrfs_is_zoned(fs_info)) {
> 	memzero_extent_buffer(eb, 0, eb->len);
> 	set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
> }
> 
> to btrfs_clean_tree_block() and then in btrfs_free_tree_block() make sure we 
> always pin the extent if we're zoned.  Thanks,

As much as I'd love the simple solution you described it unfortunately didn't work
in our testing [1]. So unless I did something completely stupid [2] (which always
is an option) I don't think we can go with the easy solution here, unfortunately.

[1]
btrfs/001       [   16.212869] BTRFS error (device nullb1): unable to find ref byte nr 805388288 parent 805388288 root 5  owner 0 offset 0
[   16.214314] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry
[   16.215270] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry                                                                                    
[   16.266987] BTRFS error (device nullb1): unable to find ref byte nr 805388288 parent 805388288 root 5  owner 0 offset 0                                
[   16.268788] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry                                           
[   16.269973] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry
[   16.271210] BTRFS: error (device nullb1) in cleanup_transaction:1904: errno=-2 No such entry                                                  
[   16.297358] BTRFS error (device nullb1): unable to find ref byte nr 805502976 parent 805502976 root 1  owner 0 offset 0                       
[   16.299128] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry   
[   16.300324] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry
[   16.335600] BTRFS error (device nullb1): unable to find ref byte nr 805388288 parent 805388288 root 5  owner 0 offset 0
[   16.337600] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry
[   16.338785] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry                                                                                    
[   16.340026] BTRFS: error (device nullb1) in cleanup_transaction:1904: errno=-2 No such entry            
[   16.366461] BTRFS error (device nullb0): unable to find ref byte nr 805502976 parent 805502976 root 1  owner 0 offset 0
[   16.368304] BTRFS: error (device nullb0) in __btrfs_free_extent:3196: errno=-2 No such entry
[   16.369480] BTRFS: error (device nullb0) in btrfs_run_delayed_refs:2215: errno=-2 No such entry
[   16.370689] BTRFS: error (device nullb0) in cleanup_transaction:1904: errno=-2 No such entry
[   16.392890] BTRFS error (device nullb1): unable to find ref byte nr 805388288 parent 805388288 root 5  owner 0 offset 0
[   16.394531] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry
[   16.395612] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry                                                        
[   16.396648] BTRFS: error (device nullb1) in cleanup_transaction:1904: errno=-2 No such entry
[   16.422315] BTRFS error (device nullb1): unable to find ref byte nr 805502976 parent 805502976 root 1  owner 0 offset 0                
[   16.424147] BTRFS: error (device nullb1) in __btrfs_free_extent:3196: errno=-2 No such entry
[   16.425320] BTRFS: error (device nullb1) in btrfs_run_delayed_refs:2215: errno=-2 No such entry
[   16.426532] BTRFS: error (device nullb1) in cleanup_transaction:1904: errno=-2 No such entry
_check_dmesg: something found in dmesg (see /home/johannes/src/xfstests-dev/results//btrfs/001.dmesg)
- output mismatch (see /home/johannes/src/xfstests-dev/results//btrfs/001.out.bad)
    --- tests/btrfs/001.out     2020-01-07 15:49:53.000000000 +0000
    +++ /home/johannes/src/xfstests-dev/results//btrfs/001.out.bad      2020-11-05 16:17:37.266632915 +0000
    @@ -3,38 +3,29 @@
     List root dir
     foo
     Creating snapshot of root dir
    +ERROR: cannot snapshot '/mnt/scratch': No such file or directory
     Create a snapshot of 'SCRATCH_MNT' in 'SCRATCH_MNT/snap'
     List root dir after snapshot
    -foo
    ...
    (Run 'diff -u /home/johannes/src/xfstests-dev/tests/btrfs/001.out /home/johannes/src/xfstests-dev/results//btrfs/001.out.bad'  to see the entire diff)

[2]
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index e896dd564434..e1bf3b561b45 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1013,7 +1013,11 @@ struct extent_buffer *read_tree_block(struct btrfs_fs_info *fs_info, u64 bytenr,
 void btrfs_clean_tree_block(struct extent_buffer *buf)
 {
        struct btrfs_fs_info *fs_info = buf->fs_info;
-       if (btrfs_header_generation(buf) ==
+
+       if (btrfs_is_zoned(fs_info)) {
+               memzero_extent_buffer(buf, 0, buf->len);
+               set_bit(EXTENT_BUFFER_NO_CHECK, &buf->bflags);
+       } else if (btrfs_header_generation(buf) ==
            fs_info->running_transaction->transid) {
                btrfs_assert_tree_locked(buf);
 
@@ -4639,8 +4643,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans,
                                     EXTENT_DIRTY);
        btrfs_destroy_pinned_extent(fs_info, &cur_trans->pinned_extents);
 
-       btrfs_free_redirty_list(cur_trans);
-
        cur_trans->state =TRANS_STATE_COMPLETED;
        wake_up(&cur_trans->commit_wait);
 }
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index bfd7f3082037..f9f9c329fa69 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3432,15 +3432,23 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
                pin = old_ref_mod >= 0 && new_ref_mod < 0;
        }
 
+       if (btrfs_is_zoned(fs_info)) {
+               struct btrfs_block_group *cache;
+
+               cache = btrfs_lookup_block_group(fs_info, buf->start);
+               pin_down_extent(trans, cache, buf->start, buf->len, 1);
+               btrfs_put_block_group(cache);
+               pin = 0;
+               goto out;
+       }
+
        if (last_ref && btrfs_header_generation(buf) == trans->transid) {
                struct btrfs_block_group *cache;
 
                if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
                        ret = check_ref_cleanup(trans, buf->start);
-                       if (!ret) {
-                               btrfs_redirty_list_add(trans->transaction, buf);
+                       if (!ret)
                                goto out;
-                       }
                }
 
                pin = 0;
@@ -3452,13 +3460,6 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
                        goto out;
                }
 
-               if (btrfs_is_zoned(fs_info)) {
-                       btrfs_redirty_list_add(trans->transaction, buf);
-                       pin_down_extent(trans, cache, buf->start, buf->len, 1);
-                       btrfs_put_block_group(cache);
-                       goto out;
-               }
-
                WARN_ON(test_bit(EXTENT_BUFFER_DIRTY, &buf->bflags));
 
                btrfs_add_free_space(cache, buf->start, buf->len);
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index a8561536cd0d..4acc96969959 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -2348,13 +2348,6 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
                goto scrub_continue;
        }
 
-       /*
-        * At this point, we should have written all the tree blocks
-        * allocated in this transaction. So it's now safe to free the
-        * redirtyied extent buffers.
-        */
-       btrfs_free_redirty_list(cur_trans);
-
        ret = write_all_supers(fs_info, 0);
        /*
         * the super is written, we can safely allow the tree-loggers
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 15f9e8a461ee..e91bdfbb03da 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -2763,8 +2763,6 @@ static noinline int walk_down_log_tree(struct btrfs_trans_handle *trans,
                                                free_extent_buffer(next);
                                                return ret;
                                        }
-                                       btrfs_redirty_list_add(
-                                               trans->transaction, next);
                                } else {
                                        if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &next->bflags))
                                                clear_extent_buffer_dirty(next);
@@ -3313,8 +3311,6 @@ static void free_log_tree(struct btrfs_trans_handle *trans,
                          EXTENT_DIRTY | EXTENT_NEW | EXTENT_NEED_WAIT);
        extent_io_tree_release(&log->log_csum_range);
 
-       if (trans && log->node)
-               btrfs_redirty_list_add(trans->transaction, log->node);
        btrfs_put_root(log);
 }
 
diff --git a/fs/btrfs/zoned.h b/fs/btrfs/zoned.h
index 5b61500a0aa9..3140c39d3728 100644
--- a/fs/btrfs/zoned.h
+++ b/fs/btrfs/zoned.h
@@ -42,9 +42,6 @@ int btrfs_reset_device_zone(struct btrfs_device *device, u64 physical,
 int btrfs_ensure_empty_zones(struct btrfs_device *device, u64 start, u64 size);
 int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache);
 void btrfs_calc_zone_unusable(struct btrfs_block_group *cache);
-void btrfs_redirty_list_add(struct btrfs_transaction *trans,
-                           struct extent_buffer *eb);
-void btrfs_free_redirty_list(struct btrfs_transaction *trans);
 void btrfs_record_physical_zoned(struct inode *inode, u64 file_offset,
                                 struct bio *bio);
 void btrfs_rewrite_logical_zoned(struct btrfs_ordered_extent *ordered);
@@ -137,10 +134,6 @@ static inline int btrfs_load_block_group_zone_info(
 
 static inline void btrfs_calc_zone_unusable(struct btrfs_block_group *cache) { }
 
-static inline void btrfs_redirty_list_add(struct btrfs_transaction *trans,
-                                         struct extent_buffer *eb) { }
-static inline void btrfs_free_redirty_list(struct btrfs_transaction *trans) { }
-
 static inline void btrfs_record_physical_zoned(struct inode *inode,
                                               u64 file_offset, struct bio *bio)
 {






  reply	other threads:[~2020-11-06  9:11 UTC|newest]

Thread overview: 119+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-30 13:51 [PATCH v9 00/41] btrfs: zoned block device support Naohiro Aota
2020-10-30 13:51 ` [PATCH v9 01/41] block: add bio_add_zone_append_page Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 02/41] iomap: support REQ_OP_ZONE_APPEND Naohiro Aota
2020-11-02  5:34     ` Naohiro Aota
2020-11-02 16:55     ` Darrick J. Wong
2020-11-02 17:39       ` Johannes Thumshirn
2020-10-30 13:51   ` [PATCH v9 03/41] btrfs: introduce ZONED feature flag Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 04/41] btrfs: Get zone information of zoned block devices Naohiro Aota
2020-11-02 16:53     ` Josef Bacik
2020-11-02 16:58       ` Johannes Thumshirn
2020-11-02 21:07       ` Naohiro Aota
2020-11-03 12:02     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 05/41] btrfs: Check and enable ZONED mode Naohiro Aota
2020-11-03 12:13     ` David Sterba
2020-11-06  9:36       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 06/41] btrfs: introduce max_zone_append_size Naohiro Aota
2020-11-02 16:57     ` Josef Bacik
2020-11-03 12:16     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 07/41] btrfs: disallow space_cache in ZONED mode Naohiro Aota
2020-11-02 17:02     ` Josef Bacik
2020-11-02 17:37       ` Johannes Thumshirn
2020-11-03 12:48     ` David Sterba
2020-11-10 10:14       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 08/41] btrfs: disallow NODATACOW " Naohiro Aota
2020-11-02 17:05     ` Josef Bacik
2020-11-03 12:57     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 09/41] btrfs: disable fallocate " Naohiro Aota
2020-11-03 13:00     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 10/41] btrfs: disallow mixed-bg " Naohiro Aota
2020-11-03 13:01     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 11/41] btrfs: implement log-structured superblock for " Naohiro Aota
2020-11-02 18:22     ` Josef Bacik
2020-11-02 18:53       ` Johannes Thumshirn
2020-11-02 19:01         ` Josef Bacik
2020-11-02 19:31           ` Johannes Thumshirn
2020-11-03  8:21       ` Naohiro Aota
2020-11-02 18:54     ` Josef Bacik
2020-11-03  3:31       ` Naohiro Aota
2020-11-03 13:15     ` David Sterba
2020-11-03 14:10     ` David Sterba
2020-11-06 10:37       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 12/41] btrfs: implement zoned chunk allocator Naohiro Aota
2020-11-02 20:09     ` Josef Bacik
2020-11-02 22:21       ` Naohiro Aota
2020-11-03 13:23     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 13/41] btrfs: verify device extent is aligned to zone Naohiro Aota
2020-11-02 20:14     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 14/41] btrfs: load zone's alloction offset Naohiro Aota
2020-11-02 20:25     ` Josef Bacik
2020-11-02 20:29       ` Josef Bacik
2020-11-02 22:43         ` Naohiro Aota
2020-11-02 22:40       ` Naohiro Aota
2020-11-03 13:28     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 15/41] btrfs: emulate write pointer for conventional zones Naohiro Aota
2020-11-02 20:37     ` Josef Bacik
2020-11-03  1:25       ` Naohiro Aota
2020-11-03 13:32     ` David Sterba
2020-10-30 13:51   ` [PATCH v9 16/41] btrfs: track unusable bytes for zones Naohiro Aota
2020-11-03 14:25     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 17/41] btrfs: do sequential extent allocation in ZONED mode Naohiro Aota
2020-11-03 14:28     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 18/41] btrfs: reset zones of unused block groups Naohiro Aota
2020-11-03 14:34     ` Josef Bacik
2020-11-10 10:40       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 19/41] btrfs: redirty released extent buffers in ZONED mode Naohiro Aota
2020-11-03 14:41     ` Josef Bacik
2020-11-06  9:11       ` Johannes Thumshirn [this message]
2020-11-06 15:01         ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 20/41] btrfs: extract page adding function Naohiro Aota
2020-11-03 14:45     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 21/41] btrfs: use bio_add_zone_append_page for zoned btrfs Naohiro Aota
2020-11-03 14:55     ` Josef Bacik
2020-11-10 10:42       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 22/41] btrfs: handle REQ_OP_ZONE_APPEND as writing Naohiro Aota
2020-11-03 14:57     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 23/41] btrfs: split ordered extent when bio is sent Naohiro Aota
2020-11-03 15:29     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 24/41] btrfs: extend btrfs_rmap_block for specifying a device Naohiro Aota
2020-11-03 15:32     ` Josef Bacik
2020-11-06 10:52       ` Johannes Thumshirn
2020-10-30 13:51   ` [PATCH v9 25/41] btrfs: use ZONE_APPEND write for ZONED btrfs Naohiro Aota
2020-11-03 15:55     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 26/41] btrfs: enable zone append writing for direct IO Naohiro Aota
2020-11-03 15:56     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 27/41] btrfs: introduce dedicated data write path for ZONED mode Naohiro Aota
2020-11-03 15:57     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 28/41] btrfs: serialize meta IOs on " Naohiro Aota
2020-11-03 16:04     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 29/41] btrfs: wait existing extents before truncating Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 30/41] btrfs: avoid async metadata checksum on ZONED mode Naohiro Aota
2020-11-03 16:05     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 31/41] btrfs: mark block groups to copy for device-replace Naohiro Aota
2020-11-03 17:09     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 32/41] btrfs: implement cloning for ZONED device-replace Naohiro Aota
2020-11-03 17:15     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 33/41] btrfs: implement copying " Naohiro Aota
2020-11-03 17:19     ` Josef Bacik
2020-11-10 11:09       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 34/41] btrfs: support dev-replace in ZONED mode Naohiro Aota
2020-11-03 20:34     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 35/41] btrfs: enable relocation " Naohiro Aota
2020-11-03 20:39     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 36/41] btrfs: relocate block group to repair IO failure in ZONED Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 37/41] btrfs: split alloc_log_tree() Naohiro Aota
2020-11-03 20:42     ` Josef Bacik
2020-11-03 22:10       ` Amy Parker
2020-11-10 11:12         ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 38/41] btrfs: extend zoned allocator to use dedicated tree-log block group Naohiro Aota
2020-11-03 20:47     ` Josef Bacik
2020-11-10  6:37       ` Naohiro Aota
2020-10-30 13:51   ` [PATCH v9 39/41] btrfs: serialize log transaction on ZONED mode Naohiro Aota
2020-11-03 20:49     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 40/41] btrfs: reorder log node allocation Naohiro Aota
2020-11-03 20:49     ` Josef Bacik
2020-10-30 13:51   ` [PATCH v9 41/41] btrfs: enable to mount ZONED incompat flag Naohiro Aota
2020-10-31  3:40   ` [PATCH v9 01/41] block: add bio_add_zone_append_page Jens Axboe
2020-11-02  5:15     ` Naohiro Aota
2020-11-02  8:24     ` Johannes Thumshirn
2020-11-03 11:54 ` [PATCH v9 00/41] btrfs: zoned block device support David Sterba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=SN4PR0401MB35981D8500BE7FAF931F1AEE9BED0@SN4PR0401MB3598.namprd04.prod.outlook.com \
    --to=johannes.thumshirn@wdc.com \
    --cc=Naohiro.Aota@wdc.com \
    --cc=dsterba@suse.com \
    --cc=hare@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.