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 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.