Skip to contentSkip to conversation

aio: avoid using queue_delayed_work in aio_kick_handler to schedule itself

PATCH In review Nikanth Karthikesan 3 participants 5 messages Jun 26, 2008 – Jul 7, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 aio: avoid using queue_delayed_work in aio_kick_handler to schedule itself

Avoid using queue_delayed_work in aio_kick_handler() to run itself immediately. Instead use aio_run_all_iocbs()

---

fs/aio.c +2 -10
index e27f611..7817e8f 100644
@@ -823,21 +823,13 @@ static void aio_kick_handler(struct work_struct *work)
 	struct kioctx *ctx = container_of(work, struct kioctx, wq.work);
 	mm_segment_t oldfs = get_fs();
 	struct mm_struct *mm;
-	int requeue;
 
 	set_fs(USER_DS);
-	use_mm(ctx->mm);
-	spin_lock_irq(&ctx->ctx_lock);
-	requeue =__aio_run_iocbs(ctx);
 	mm = ctx->mm;
-	spin_unlock_irq(&ctx->ctx_lock);
+	use_mm(mm);
+	aio_run_all_iocbs(ctx);
  	unuse_mm(mm);
 	set_fs(oldfs);
-	/*
-	 * we're in a worker thread already, don't use queue_delayed_work,
-	 */
-	if (requeue)
-		queue_delayed_work(aio_wq, &ctx->wq, 0);
 }
 
 
#1 lore raw
Quoted 2 lines · Nikanth Karthikesan

Avoid using queue_delayed_work in aio_kick_handler() to run itself immediately. Instead use aio_run_all_iocbs()

Can you give some rationale for this change? Also, how did you test it?

Cheers,

Jeff

#2 lore raw
Quoted 6 lines · On Friday 27 June 2008 18:41:37 Jeff Moyer wrote:
Quoted 2 lines · Nikanth Karthikesan

Avoid using queue_delayed_work in aio_kick_handler() to run itself immediately. Instead use aio_run_all_iocbs()

Can you give some rationale for this change? Also, how did you test it?

The comment in aio_kick_handler(), "we're in a worker thread already, don't use queue_delayed_work" triggered me to do this. Ran multiple instances of Stephen Hemminger's aio cp for basic testing.

I think this would make it unfair between different kioctx? May be only the comment should be removed?

Thanks
Nikanth Karthikesan

#3 lore raw
Quoted 5 lines · On Monday 30 June 2008 11:20:27 Nikanth Karthikesan wrote:
Quoted 4 lines · On Friday 27 June 2008 18:41:37 Jeff Moyer wrote:
Quoted 2 lines · Nikanth Karthikesan

Avoid using queue_delayed_work in aio_kick_handler() to run itself immediately. Instead use aio_run_all_iocbs()

Quoted 9 lines
Quoted 1 line

Can you give some rationale for this change? Also, how did you test it?

The comment in aio_kick_handler(), "we're in a worker thread already, don't use queue_delayed_work" triggered me to do this. Ran multiple instances of Stephen Hemminger's aio cp for basic testing.

I think this would make it unfair between different kioctx? May be only the comment should be removed?

Anyway we are scheduling it again with timeout of 0, so fairness shouldnt be a problem? If yes, wouldn't this change reduce the overhead of queuing the work again, and repeated lock/unlock.

Thanks
Nikanth Karthikesan

#4 lore raw
Quoted 12 lines · Nikanth Karthikesan
Quoted 11 lines · On Monday 30 June 2008 11:20:27 Nikanth Karthikesan wrote:
Quoted 5 lines · On Friday 27 June 2008 18:41:37 Jeff Moyer wrote:
Quoted 2 lines · Nikanth Karthikesan

Avoid using queue_delayed_work in aio_kick_handler() to run itself immediately. Instead use aio_run_all_iocbs()

Can you give some rationale for this change? Also, how did you test it?

The comment in aio_kick_handler(), "we're in a worker thread already, don't use queue_delayed_work" triggered me to do this. Ran multiple instances of Stephen Hemminger's aio cp for basic testing.

Quoted 7 lines
Quoted 3 lines

I think this would make it unfair between different kioctx? May be only the comment should be removed?

Anyway we are scheduling it again with timeout of 0, so fairness shouldnt be a problem? If yes, wouldn't this change reduce the overhead of queuing the work again, and repeated lock/unlock.

The main purpose of the delayed queuing was to improve performance with workloads where the work was more or less immediately done. The best example is aio pipes. The delayed queue cut down on the context switch rate dramatically and made a large performance improvement for pipe workloads (without cutting down on other perf at the time).

-chris