Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
From: Naohiro Aota <Naohiro.Aota@wdc.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	"David Sterba" <dsterba@suse.com>, "Chris Mason" <clm@fb.com>,
	"Qu Wenruo" <wqu@suse.com>, "Nikolay Borisov" <nborisov@suse.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Hannes Reinecke" <hare@suse.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	"Damien Le Moal" <Damien.LeMoal@wdc.com>,
	"Matias Bjørling" <mb@lightnvm.io>,
	"Johannes Thumshirn" <jthumshirn@suse.de>,
	"Bart Van Assche" <bvanassche@acm.org>
Subject: Re: [PATCH 14/19] btrfs: redirty released extent buffers in sequential BGs
Date: Tue, 18 Jun 2019 09:09:34 +0000
Message-ID: <SN6PR04MB523180783547B7CEC55285968CEA0@SN6PR04MB5231.namprd04.prod.outlook.com> (raw)
In-Reply-To: <20190613142408.p3ra5urczrzgqr2q@MacBook-Pro-91.local>

On 2019/06/13 23:24, Josef Bacik wrote:
> On Fri, Jun 07, 2019 at 10:10:20PM +0900, Naohiro Aota wrote:
>> Tree manipulating operations like merging nodes often release
>> once-allocated tree nodes. Btrfs cleans such nodes so that pages in the
>> node are not uselessly written out. On HMZONED drives, however, such
>> optimization blocks the following IOs as the cancellation of the write out
>> of the freed blocks breaks the sequential write sequence expected by the
>> device.
>>
>> This patch introduces a list of clean extent buffers that have been
>> released in a transaction. Btrfs consult the list before writing out and
>> waiting for the IOs, and it redirties a buffer if 1) it's in sequential BG,
>> 2) it's in un-submit range, and 3) it's not under IO. Thus, such buffers
>> are marked for IO in btrfs_write_and_wait_transaction() to send proper bios
>> to the disk.
>>
>> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
>> ---
>>   fs/btrfs/disk-io.c     | 27 ++++++++++++++++++++++++---
>>   fs/btrfs/extent_io.c   |  1 +
>>   fs/btrfs/extent_io.h   |  2 ++
>>   fs/btrfs/transaction.c | 35 +++++++++++++++++++++++++++++++++++
>>   fs/btrfs/transaction.h |  3 +++
>>   5 files changed, 65 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 6651986da470..c6147fce648f 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -535,7 +535,9 @@ static int csum_dirty_buffer(struct btrfs_fs_info *fs_info, struct page *page)
>>   	if (csum_tree_block(eb, result))
>>   		return -EINVAL;
>>   
>> -	if (btrfs_header_level(eb))
>> +	if (test_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags))
>> +		ret = 0;
>> +	else if (btrfs_header_level(eb))
>>   		ret = btrfs_check_node(eb);
>>   	else
>>   		ret = btrfs_check_leaf_full(eb);
>> @@ -1115,10 +1117,20 @@ 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) ==
>> -	    fs_info->running_transaction->transid) {
>> +	struct btrfs_transaction *cur_trans = fs_info->running_transaction;
>> +
>> +	if (btrfs_header_generation(buf) == cur_trans->transid) {
>>   		btrfs_assert_tree_locked(buf);
>>   
>> +		if (btrfs_fs_incompat(fs_info, HMZONED) &&
>> +		    list_empty(&buf->release_list)) {
>> +			atomic_inc(&buf->refs);
>> +			spin_lock(&cur_trans->releasing_ebs_lock);
>> +			list_add_tail(&buf->release_list,
>> +				      &cur_trans->releasing_ebs);
>> +			spin_unlock(&cur_trans->releasing_ebs_lock);
>> +		}
>> +
>>   		if (test_and_clear_bit(EXTENT_BUFFER_DIRTY, &buf->bflags)) {
>>   			percpu_counter_add_batch(&fs_info->dirty_metadata_bytes,
>>   						 -buf->len,
>> @@ -4533,6 +4545,15 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans,
>>   	btrfs_destroy_pinned_extent(fs_info,
>>   				    fs_info->pinned_extents);
>>   
>> +	while (!list_empty(&cur_trans->releasing_ebs)) {
>> +		struct extent_buffer *eb;
>> +
>> +		eb = list_first_entry(&cur_trans->releasing_ebs,
>> +				      struct extent_buffer, release_list);
>> +		list_del_init(&eb->release_list);
>> +		free_extent_buffer(eb);
>> +	}
>> +
>>   	cur_trans->state =TRANS_STATE_COMPLETED;
>>   	wake_up(&cur_trans->commit_wait);
>>   }
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 13fca7bfc1f2..c73c69e2bef4 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -4816,6 +4816,7 @@ __alloc_extent_buffer(struct btrfs_fs_info *fs_info, u64 start,
>>   	init_waitqueue_head(&eb->read_lock_wq);
>>   
>>   	btrfs_leak_debug_add(&eb->leak_list, &buffers);
>> +	INIT_LIST_HEAD(&eb->release_list);
>>   
>>   	spin_lock_init(&eb->refs_lock);
>>   	atomic_set(&eb->refs, 1);
>> diff --git a/fs/btrfs/extent_io.h b/fs/btrfs/extent_io.h
>> index aa18a16a6ed7..2987a01f84f9 100644
>> --- a/fs/btrfs/extent_io.h
>> +++ b/fs/btrfs/extent_io.h
>> @@ -58,6 +58,7 @@ enum {
>>   	EXTENT_BUFFER_IN_TREE,
>>   	/* write IO error */
>>   	EXTENT_BUFFER_WRITE_ERR,
>> +	EXTENT_BUFFER_NO_CHECK,
>>   };
>>   
>>   /* these are flags for __process_pages_contig */
>> @@ -186,6 +187,7 @@ struct extent_buffer {
>>   	 */
>>   	wait_queue_head_t read_lock_wq;
>>   	struct page *pages[INLINE_EXTENT_BUFFER_PAGES];
>> +	struct list_head release_list;
>>   #ifdef CONFIG_BTRFS_DEBUG
>>   	atomic_t spinning_writers;
>>   	atomic_t spinning_readers;
>> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
>> index 3f6811cdf803..ded40ad75419 100644
>> --- a/fs/btrfs/transaction.c
>> +++ b/fs/btrfs/transaction.c
>> @@ -236,6 +236,8 @@ static noinline int join_transaction(struct btrfs_fs_info *fs_info,
>>   	spin_lock_init(&cur_trans->dirty_bgs_lock);
>>   	INIT_LIST_HEAD(&cur_trans->deleted_bgs);
>>   	spin_lock_init(&cur_trans->dropped_roots_lock);
>> +	INIT_LIST_HEAD(&cur_trans->releasing_ebs);
>> +	spin_lock_init(&cur_trans->releasing_ebs_lock);
>>   	list_add_tail(&cur_trans->list, &fs_info->trans_list);
>>   	extent_io_tree_init(fs_info, &cur_trans->dirty_pages,
>>   			IO_TREE_TRANS_DIRTY_PAGES, fs_info->btree_inode);
>> @@ -2219,7 +2221,31 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>>   
>>   	wake_up(&fs_info->transaction_wait);
>>   
>> +	if (btrfs_fs_incompat(fs_info, HMZONED)) {
>> +		struct extent_buffer *eb;
>> +
>> +		list_for_each_entry(eb, &cur_trans->releasing_ebs,
>> +				    release_list) {
>> +			struct btrfs_block_group_cache *cache;
>> +
>> +			cache = btrfs_lookup_block_group(fs_info, eb->start);
>> +			if (!cache)
>> +				continue;
>> +			mutex_lock(&cache->submit_lock);
>> +			if (cache->alloc_type == BTRFS_ALLOC_SEQ &&
>> +			    cache->submit_offset <= eb->start &&
>> +			    !extent_buffer_under_io(eb)) {
>> +				set_extent_buffer_dirty(eb);
>> +				cache->space_info->bytes_readonly += eb->len;
> 
> Huh?
> 

I'm tracking once allocated then freed region in "space_info->bytes_readonly".
As I wrote in the other reply, I can add and use "space_info->bytes_zone_unavailable" instead.

>> +				set_bit(EXTENT_BUFFER_NO_CHECK, &eb->bflags);
>> +			}
>> +			mutex_unlock(&cache->submit_lock);
>> +			btrfs_put_block_group(cache);
>> +		}
>> +	}
>> +
> 
> Helper here please.
>>   	ret = btrfs_write_and_wait_transaction(trans);
>> +
>>   	if (ret) {
>>   		btrfs_handle_fs_error(fs_info, ret,
>>   				      "Error while writing out transaction");
>> @@ -2227,6 +2253,15 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
>>   		goto scrub_continue;
>>   	}
>>   
>> +	while (!list_empty(&cur_trans->releasing_ebs)) {
>> +		struct extent_buffer *eb;
>> +
>> +		eb = list_first_entry(&cur_trans->releasing_ebs,
>> +				      struct extent_buffer, release_list);
>> +		list_del_init(&eb->release_list);
>> +		free_extent_buffer(eb);
>> +	}
>> +
> 
> Another helper, and also can't we release eb's above that we didn't need to
> re-mark dirty?  Thanks,
> 
> Josef
> 

hm, we can do so. I'll change the code in the next version.
Thanks,

  reply index

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-07 13:10 [PATCH v2 00/19] btrfs zoned block device support Naohiro Aota
2019-06-07 13:10 ` [PATCH 01/19] btrfs: introduce HMZONED feature flag Naohiro Aota
2019-06-07 13:10 ` [PATCH 02/19] btrfs: Get zone information of zoned block devices Naohiro Aota
2019-06-13 13:58   ` Josef Bacik
2019-06-18  6:04     ` Naohiro Aota
2019-06-13 13:58   ` Josef Bacik
2019-06-17 18:57   ` David Sterba
2019-06-18  6:42     ` Naohiro Aota
2019-06-27 15:11       ` David Sterba
2019-06-07 13:10 ` [PATCH 03/19] btrfs: Check and enable HMZONED mode Naohiro Aota
2019-06-13 13:57   ` Josef Bacik
2019-06-18  6:43     ` Naohiro Aota
2019-06-07 13:10 ` [PATCH 04/19] btrfs: disable fallocate in " Naohiro Aota
2019-06-07 13:10 ` [PATCH 05/19] btrfs: disable direct IO " Naohiro Aota
2019-06-13 14:00   ` Josef Bacik
2019-06-18  8:17     ` Naohiro Aota
2019-06-07 13:10 ` [PATCH 06/19] btrfs: align dev extent allocation to zone boundary Naohiro Aota
2019-06-07 13:10 ` [PATCH 07/19] btrfs: do sequential extent allocation in HMZONED mode Naohiro Aota
2019-06-13 14:07   ` Josef Bacik
2019-06-18  8:28     ` Naohiro Aota
2019-06-18 13:37       ` Josef Bacik
2019-06-17 22:30   ` David Sterba
2019-06-18  8:49     ` Naohiro Aota
2019-06-27 15:28       ` David Sterba
2019-06-07 13:10 ` [PATCH 08/19] btrfs: make unmirroed BGs readonly only if we have at least one writable BG Naohiro Aota
2019-06-13 14:09   ` Josef Bacik
2019-06-18  7:42     ` Naohiro Aota
2019-06-18 13:35       ` Josef Bacik
2019-06-07 13:10 ` [PATCH 09/19] btrfs: limit super block locations in HMZONED mode Naohiro Aota
2019-06-13 14:12   ` Josef Bacik
2019-06-18  8:51     ` Naohiro Aota
2019-06-17 22:53   ` David Sterba
2019-06-18  9:01     ` Naohiro Aota
2019-06-27 15:35       ` David Sterba
2019-06-28  3:55   ` Anand Jain
2019-06-28  6:39     ` Naohiro Aota
2019-06-28  6:52       ` Anand Jain
2019-06-07 13:10 ` [PATCH 10/19] btrfs: rename btrfs_map_bio() Naohiro Aota
2019-06-07 13:10 ` [PATCH 11/19] btrfs: introduce submit buffer Naohiro Aota
2019-06-13 14:14   ` Josef Bacik
2019-06-17  3:16     ` Damien Le Moal
2019-06-18  0:00       ` David Sterba
2019-06-18  4:04         ` Damien Le Moal
2019-06-18 13:33       ` Josef Bacik
2019-06-19 10:32         ` Damien Le Moal
2019-06-07 13:10 ` [PATCH 12/19] btrfs: expire submit buffer on timeout Naohiro Aota
2019-06-13 14:15   ` Josef Bacik
2019-06-17  3:19     ` Damien Le Moal
2019-06-07 13:10 ` [PATCH 13/19] btrfs: avoid sync IO prioritization on checksum in HMZONED mode Naohiro Aota
2019-06-13 14:17   ` Josef Bacik
2019-06-07 13:10 ` [PATCH 14/19] btrfs: redirty released extent buffers in sequential BGs Naohiro Aota
2019-06-13 14:24   ` Josef Bacik
2019-06-18  9:09     ` Naohiro Aota [this message]
2019-06-07 13:10 ` [PATCH 15/19] btrfs: reset zones of unused block groups Naohiro Aota
2019-06-07 13:10 ` [PATCH 16/19] btrfs: wait existing extents before truncating Naohiro Aota
2019-06-13 14:25   ` Josef Bacik
2019-06-07 13:10 ` [PATCH 17/19] btrfs: shrink delayed allocation size in HMZONED mode Naohiro Aota
2019-06-13 14:27   ` Josef Bacik
2019-06-07 13:10 ` [PATCH 18/19] btrfs: support dev-replace " Naohiro Aota
2019-06-13 14:33   ` Josef Bacik
2019-06-18  9:14     ` Naohiro Aota
2019-06-07 13:10 ` [PATCH 19/19] btrfs: enable to mount HMZONED incompat flag Naohiro Aota
2019-06-07 13:17 ` [PATCH 01/12] btrfs-progs: build: Check zoned block device support Naohiro Aota
2019-06-07 13:17   ` [PATCH 02/12] btrfs-progs: utils: Introduce queue_param Naohiro Aota
2019-06-07 13:17   ` [PATCH 03/12] btrfs-progs: add new HMZONED feature flag Naohiro Aota
2019-06-07 13:17   ` [PATCH 04/12] btrfs-progs: Introduce zone block device helper functions Naohiro Aota
2019-06-07 13:17   ` [PATCH 05/12] btrfs-progs: load and check zone information Naohiro Aota
2019-06-07 13:17   ` [PATCH 06/12] btrfs-progs: avoid writing super block to sequential zones Naohiro Aota
2019-06-07 13:17   ` [PATCH 07/12] btrfs-progs: support discarding zoned device Naohiro Aota
2019-06-07 13:17   ` [PATCH 08/12] btrfs-progs: volume: align chunk allocation to zones Naohiro Aota
2019-06-07 13:17   ` [PATCH 09/12] btrfs-progs: do sequential allocation Naohiro Aota
2019-06-07 13:17   ` [PATCH 10/12] btrfs-progs: mkfs: Zoned block device support Naohiro Aota
2019-06-07 13:17   ` [PATCH 11/12] btrfs-progs: device-add: support HMZONED device Naohiro Aota
2019-06-07 13:17   ` [PATCH 12/12] btrfs-progs: introduce support for dev-place " Naohiro Aota
2019-06-12 17:51 ` [PATCH v2 00/19] btrfs zoned block device support David Sterba
2019-06-13  4:59   ` Naohiro Aota
2019-06-13 13:46     ` David Sterba
2019-06-14  2:07       ` Naohiro Aota
2019-06-17  2:44       ` Damien Le Moal

Reply instructions:

You may reply publically 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=SN6PR04MB523180783547B7CEC55285968CEA0@SN6PR04MB5231.namprd04.prod.outlook.com \
    --to=naohiro.aota@wdc.com \
    --cc=Damien.LeMoal@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=hare@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=jthumshirn@suse.de \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mb@lightnvm.io \
    --cc=nborisov@suse.com \
    --cc=wqu@suse.com \
    /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

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org linux-btrfs@archiver.kernel.org
	public-inbox-index linux-btrfs


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/ public-inbox