From: Liu Bo <bo.li.liu@oracle.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 9/9] btrfs: Replace thread_pool_size with workqueue default value
Date: Fri, 13 Sep 2013 09:47:59 +0800 [thread overview]
Message-ID: <20130913014758.GD7505@localhost.localdomain> (raw)
In-Reply-To: <1378973304-11693-10-git-send-email-quwenruo@cn.fujitsu.com>
On Thu, Sep 12, 2013 at 04:08:24PM +0800, Qu Wenruo wrote:
> The original btrfs_workers uses the fs_info->thread_pool_size as the
> max_active, and the previous patches followed this way.
>
> But the kernel workqueue has the default value(0) for workqueue,
> and workqueue itself has some threshold mechanism to prevent creating
> too many threads, so we should use the default value.
>
> Since the thread_pool_size algorithm is not used, related codes should
> also be changed.
Ohh, I should have seen this mail first before commenting 'max_active'.
I think that some tuning work should be done on this part, according to
my tests, setting max_active=0 will create ~258 worker helpers
(kworker/uX:X if you set WQ_UNBOUND), this may cause too many context
switches which will have an impact on performance in some cases.
-liubo
>
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> fs/btrfs/disk-io.c | 12 +++++++-----
> fs/btrfs/super.c | 3 +--
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index a61e1fe..0446d27 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -750,9 +750,11 @@ int btrfs_bio_wq_end_io(struct btrfs_fs_info *info, struct bio *bio,
>
> unsigned long btrfs_async_submit_limit(struct btrfs_fs_info *info)
> {
> - unsigned long limit = min_t(unsigned long,
> - info->thread_pool_size,
> - info->fs_devices->open_devices);
> + unsigned long limit;
> + limit = info->thread_pool_size ?
> + min_t(unsigned long, info->thread_pool_size,
> + info->fs_devices->open_devices) :
> + info->fs_devices->open_devices;
> return 256 * limit;
> }
>
> @@ -2191,8 +2193,8 @@ int open_ctree(struct super_block *sb,
> INIT_RADIX_TREE(&fs_info->reada_tree, GFP_NOFS & ~__GFP_WAIT);
> spin_lock_init(&fs_info->reada_lock);
>
> - fs_info->thread_pool_size = min_t(unsigned long,
> - num_online_cpus() + 2, 8);
> + /* use the default value of kernel workqueue */
> + fs_info->thread_pool_size = 0;
>
> INIT_LIST_HEAD(&fs_info->ordered_roots);
> spin_lock_init(&fs_info->ordered_root_lock);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 63e653c..ccf412f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -898,8 +898,7 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
> if (info->alloc_start != 0)
> seq_printf(seq, ",alloc_start=%llu",
> (unsigned long long)info->alloc_start);
> - if (info->thread_pool_size != min_t(unsigned long,
> - num_online_cpus() + 2, 8))
> + if (info->thread_pool_size)
> seq_printf(seq, ",thread_pool=%d", info->thread_pool_size);
> if (btrfs_test_opt(root, COMPRESS)) {
> if (info->compress_type == BTRFS_COMPRESS_ZLIB)
> --
> 1.8.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-09-13 1:48 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-12 8:08 [PATCH v2 0/9] btrfs: Replace the btrfs_workers with kernel workqueue Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 1/9] btrfs: Cleanup the unused struct async_sched Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 2/9] btrfs: use kernel workqueue to replace the btrfs_workers functions Qu Wenruo
2013-09-13 1:29 ` Liu Bo
2013-09-13 1:45 ` Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 3/9] btrfs: Added btrfs_workqueue_struct implemented ordered execution based on kernel workqueue Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 4/9] btrfs: Add high priority workqueue support for btrfs_workqueue_struct Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 5/9] btrfs: Use btrfs_workqueue_struct to replace the fs_info->workers Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 6/9] btrfs: Use btrfs_workqueue_struct to replace the fs_info->delalloc_workers Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 7/9] btrfs: Replace the fs_info->submit_workers with kernel workqueue Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 8/9] btrfs: Cleanup the old btrfs workqueue Qu Wenruo
2013-09-12 8:08 ` [PATCH v2 9/9] btrfs: Replace thread_pool_size with workqueue default value Qu Wenruo
2013-09-13 1:47 ` Liu Bo [this message]
2013-09-13 3:15 ` Qu Wenruo
2013-09-17 2:41 ` Qu Wenruo
2013-09-12 17:37 ` [PATCH v2 0/9] btrfs: Replace the btrfs_workers with kernel workqueue David Sterba
2013-09-13 2:03 ` Qu Wenruo
2013-09-20 6:13 ` Qu Wenruo
2013-10-01 14:50 ` David Sterba
2013-10-02 1:50 ` Qu Wenruo
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=20130913014758.GD7505@localhost.localdomain \
--to=bo.li.liu@oracle.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=quwenruo@cn.fujitsu.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
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.