于 2013年09月13日 01:37, David Sterba 写道:
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.
Definitely needs more test since the I lack enough different disks to test with.

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
Yes, that's right.
But the problem is, even I added "btrfs-" prefix to the wq,
the real work executor is kernel workers without any prefix.
Still hard to debugging due to the workqueue mechanism.

* 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
Also true, since I didn't find a way to ensure the high priority work
to be executed before any normal priority work,
I choose this workaround.
(Seems the original btrfs_workers also have some mechanism to avoid
starving, so I think this way maybe OK)

* the idle_thresh attribute is not reflected in the new code, I don't
   know if the kernel workqueues have something equivalent
It seems that kernel will not create kthread without any control,
but still needs more investigation to make sure.


Other random comments:

* you can use the same files for the new helpers, instead of bwq.[ch]
The way I used is to avoid naming confliction and easy to clean.
If needed I'll also use the async-thread.[ch]

* btrfs_workqueue_struct can drop the _struct suffix
The naming rule is mostly copied from kernel wq and just add "btrfs_" prefix
if no naming confliction.
Will modify if needed.
* WQ_MEM_RECLAIM for the scrub thread does not seem right

* WQ_FREEZABLE should be probably set
Will modify soon.



david


Thanks for the comment.

Qu

--
-----------------------------------------------------
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: quwen...@cn.fujitsu.com
-----------------------------------------------------

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to