From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: dsterba@suse.cz, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 0/9] btrfs: Replace the btrfs_workers with kernel workqueue
Date: Fri, 20 Sep 2013 14:13:08 +0800 [thread overview]
Message-ID: <523BE774.4070808@cn.fujitsu.com> (raw)
In-Reply-To: <20130912173718.GE6810@twin.jikos.cz>
On thu, 12 Sep 2013 19:37:18 +0200, David Sterba wrote:
> On Thu, Sep 12, 2013 at 04:08:15PM +0800, Qu Wenruo wrote:
>> Use kernel workqueue and kernel workqueue based new btrfs_workqueue_struct to replace
>> the old btrfs_workers.
>> The main goal is to reduce the redundant codes(800 lines vs 200 lines) and
>> try to get benefits from the latest workqueue changes.
>>
>> About the performance, the test suite I used is bonnie++,
>> and there seems no significant regression.
> You're replacing a core infrastructure building block, more testing is
> absolutely required, but using the available infrastructure is a good
> move.
>
> I found a few things that do not replace the current implementation
> one-to-one:
>
> * the thread names lost the btrfs- prefix, this makes it hard to
> identify the processes and we want this, either debugging or
> performance monitoring
>
> * od high priority tasks were handled in threads with unchanged priority
> and just prioritized within the queue
> newly addded WQ_HIGHPRI elevates the nice level of the thread, ie.
> it's not the same thing as before -- I need to look closer
>
> * the idle_thresh attribute is not reflected in the new code, I don't
> know if the kernel workqueues have something equivalent
>
>
> Other random comments:
>
> * you can use the same files for the new helpers, instead of bwq.[ch]
>
> * btrfs_workqueue_struct can drop the _struct suffix
>
> * WQ_MEM_RECLAIM for the scrub thread does not seem right
I think scrub_workers,scrub_wr_completion_workers still need WQ_MEM_RECLAIM.
However scrub_nocow_workers does not need WQ_MEM_RECLAIM flags.
Did you mean this?
If you didn't mean this, would you please tell me why the WQ_MEM_RECLAIM
is not
needed?
Thanks
Qu
>
> * WQ_FREEZABLE should be probably set
>
>
> david
> --
> 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
>
--
-----------------------------------------------------
Qu Wenruo
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
No. 6 Wenzhu Road, Nanjing, 210012, China
TEL: +86+25-86630566-8526
COINS: 7998-8526
FAX: +86+25-83317685
MAIL: quwenruo@cn.fujitsu.com
-----------------------------------------------------
next prev parent reply other threads:[~2013-09-20 6:12 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
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 [this message]
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=523BE774.4070808@cn.fujitsu.com \
--to=quwenruo@cn.fujitsu.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).