From: Krishna Kumar <krkumar2@in.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: Oleg Nesterov <oleg@tv-sign.ru>, Krishna Kumar <krkumar2@in.ibm.com>
Subject: [REV2: PATCH 0/2] workqueue: Two API's to update delayed works quickly
Date: Mon, 29 Sep 2008 12:33:02 +0530 [thread overview]
Message-ID: <20080929070302.7386.72726.sendpatchset@localhost.localdomain> (raw)
From: Krishna Kumar <krkumar2@in.ibm.com>
Implement two API's for quickly updating delayed works:
int schedule_update_delayed_work(struct delayed_work *dwork,
unsigned long delay);
int queue_update_delayed_work(struct workqueue_struct *wq,
struct delayed_work *dwork,
unsigned long delay);
I tested the following combinations, after Oleg suggested some improvements:
A. - Original submission.
1 CPU -> Org: 474213 New: 55365 Time saved: 88.3%
4 CPU -> Org: 3650631 New: 225160 Time saved: 93.8%
B1. - Oleg's suggestion to avoid costly __cancel_work_timer.
1 CPU -> Org: 474213 New: 233714 Time saved: 50.7%
4 CPU -> Org: 3650631 New: 959455 Time saved: 73.7%
B2. - B1 + check for same expiry.
1 CPU -> Org: 474213 New: 72276 Time saved: 84.8%
4 CPU -> Org: 3650631 New: 301510 Time saved: 91.7%
C. - Merge Oleg's idea with code A.
1 CPU -> Org: 474213 New: 55160 Time saved: 88.4%
4 CPU -> Org: 3650631 New: 215369 Time saved: 94.1%
Merged code seems to perform the best, though the difference with the original
submission is only marginal. Code snippets below with comments removed:
A: Original submission:
-----------------------
void queue_update_delayed_work(struct workqueue_struct *wq,
struct delayed_work *dwork, unsigned long delay)
{
struct work_struct *work = &dwork->work;
if (likely(test_and_set_bit(WORK_STRUCT_PENDING,
work_data_bits(work)))) {
struct timer_list *timer = &dwork->timer;
if (jiffies + delay == timer->expires)
return;
__cancel_work_timer_internal(work, timer);
}
__queue_delayed_work(-1, dwork, work, wq, delay);
}
B1: Oleg suggestion:
------------------------
(I optimized a bit as parallel updates are not important, only one needs to
succeed)
int queue_update_delayed_work(struct workqueue_struct *wq,
struct delayed_work *dwork, unsigned long delay)
{
unsigned long when = jiffies + delay;
if (queue_delayed_work(wq, dwork, delay))
return 1;
if (update_timer_expiry(&dwork->timer, when))
return 0;
cancel_work_sync(&dwork->work);
queue_delayed_work(wq, dwork, delay);
return 0;
}
B2: B1 + extra check:
---------------------
int queue_update_delayed_work(struct workqueue_struct *wq,
struct delayed_work *dwork, unsigned long delay)
{
unsigned long when = jiffies + delay;
if (queue_delayed_work(wq, dwork, delay))
return 1;
if (when == dwork->timer.expires ||
update_timer_expiry(&dwork->timer, when))
return 0;
cancel_work_sync(&dwork->work);
queue_delayed_work(wq, dwork, delay);
return 0;
}
C: Combined code:
-----------------
int queue_update_delayed_work(struct workqueue_struct *wq,
struct delayed_work *dwork, unsigned long delay)
{
struct work_struct *work = &dwork->work;
if (!test_and_set_bit(WORK_STRUCT_PENDING, work_data_bits(work))) {
__queue_delayed_work(-1, dwork, work, wq, delay);
return 1;
} else {
struct timer_list *timer = &dwork->timer;
unsigned long when = jiffies + delay;
int ret;
if (timer->expires == when)
return 0;
do {
if (likely(update_timer_expiry(timer, when)))
return 0;
ret = try_to_grab_pending(work);
if (ret < 0)
wait_on_work(work);
} while (ret < 0);
__queue_delayed_work(-1, dwork, work, wq, delay);
return 0;
}
}
I am submitting the merged code. Please review and provide feedback.
Thanks
- KK
[PATCH 1/2]: Implement the kernel API's
[PATCH 2/2]: Modify some drivers to use the new APIs instead of the old ones
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
(Description of the patches from first submission:
These API's are useful to update an existing work entry more efficiently (but
can be used to queue a new work entry too) when the operation is done very
frequently. The rationale is to save time of first cancelling work/timer and
adding work/timer when the same work is added many times in quick succession.
queue_update_delayed_work_on() and schedule_update_delayed_work_on() are not
implemented as there are no users which can take advantage of these variations.
Example possible users (FS's, wireless drivers):
------------------------------------------------
lbs_scan_networks, nfs4_renew_state, ocfs2_schedule_truncate_log_flush,
nfs4_schedule_state_renewal, afs_flush_callback_breaks, afs_reap_server,
afs_purge_servers, afs_vlocation_reaper, afs_vlocation_purge,
o2hb_arm_write_timeout, lbs_scan_networks, isr_indicate_rf_kill,
lbs_postpone_association_work, isr_scan_complete, ipw_radio_kill_sw,
__ipw_led_activity_on, ipw_radio_kill_sw, ds2760_battery_resume, etc.
Performance (numbers in jiffies):
---------------------------------
These API's are useful in following cases:
a. add followed by cancel+add with long period between the add's. Time
saved is the time to clear and re-setting the WORK_STRUCT_PENDING
bit, plus the reduction of one API call.
b. add followed by cancel+add with very short time between the add's.
Time saved is all of above, plus avoid cancelling and re-adding.
)
next reply other threads:[~2008-09-29 7:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-29 7:03 Krishna Kumar [this message]
2008-09-29 7:03 ` [REV2: PATCH 1/2]: workqueue: Implement the kernel API Krishna Kumar
2008-09-29 14:27 ` Oleg Nesterov
2008-09-30 11:08 ` Krishna Kumar2
2008-09-30 13:15 ` Oleg Nesterov
2008-09-29 7:03 ` [REV2: PATCH 2/2]: workqueue: Modify some users to use the new API Krishna Kumar
2008-09-29 14:45 ` Oleg Nesterov
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=20080929070302.7386.72726.sendpatchset@localhost.localdomain \
--to=krkumar2@in.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg@tv-sign.ru \
/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