ext3/4: enhance fsync performance when using cfq
in [Kernel]
|
Prev: [Bug #15518] CONFIG_NO_BOOTMEM=y breaks boot on 32bit
Next: [Bug #15655] corrupt ext3 fs and partial freeze
From: Jens Axboe on 8 Apr 2010 10:20 On Thu, Apr 08 2010, Vivek Goyal wrote: > On Thu, Apr 08, 2010 at 01:04:42PM +0200, Jens Axboe wrote: > > On Wed, Apr 07 2010, Vivek Goyal wrote: > > > On Wed, Apr 07, 2010 at 05:18:12PM -0400, Jeff Moyer wrote: > > > > Hi again, > > > > > > > > So, here's another stab at fixing this. This patch is very much an RFC, > > > > so do not pull it into anything bound for Linus. ;-) For those new to > > > > this topic, here is the original posting: http://lkml.org/lkml/2010/4/1/344 > > > > > > > > The basic problem is that, when running iozone on smallish files (up to > > > > 8MB in size) and including fsync in the timings, deadline outperforms > > > > CFQ by a factor of about 5 for 64KB files, and by about 10% for 8MB > > > > files. From examining the blktrace data, it appears that iozone will > > > > issue an fsync() call, and will have to wait until it's CFQ timeslice > > > > has expired before the journal thread can run to actually commit data to > > > > disk. > > > > > > > > The approach below puts an explicit call into the filesystem-specific > > > > fsync code to yield the disk so that the jbd[2] process has a chance to > > > > issue I/O. This bring performance of CFQ in line with deadline. > > > > > > > > There is one outstanding issue with the patch that Vivek pointed out. > > > > Basically, this could starve out the sync-noidle workload if there is a > > > > lot of fsync-ing going on. I'll address that in a follow-on patch. For > > > > now, I wanted to get the idea out there for others to comment on. > > > > > > > > Thanks a ton to Vivek for spotting the problem with the initial > > > > approach, and for his continued review. > > > > > > > > > > Thanks Jeff. Conceptually this appraoch makes lot of sense to me. Higher > > > layers explicitly telling CFQ not to idle/yield the slice. > > > > > > My firefox timing test is perfoming much better now. > > > > > > real 0m15.957s > > > user 0m0.608s > > > sys 0m0.165s > > > > > > real 0m12.984s > > > user 0m0.602s > > > sys 0m0.148s > > > > > > real 0m13.057s > > > user 0m0.624s > > > sys 0m0.145s > > > > > > So we got to take care of two issues now. > > > > > > - Make it work with dm/md devices also. Somehow shall have to propogate > > > this yield semantic down the stack. > > > > The way that Jeff set it up, it's completely parallel to eg congestion > > or unplugging. So that should be easily doable. > > > > Ok, so various dm targets now need to define "yield_fn" and propogate the > yield call to all the component devices. Exactly. > > > - The issue of making sure we don't yield if we are servicing sync-noidle > > > tree and there is other IO going on which relies on sync-noidle tree > > > idling (as you have already mentioned). > > > > Agree. > > > > > > + /* > > > > + * This is primarily called to ensure that the long synchronous > > > > + * time slice does not prevent other I/O happenning (like journal > > > > + * commits) while we idle waiting for it. Thus, check to see if the > > > > + * current cfqq is the sync cfqq for this process. > > > > + */ > > > > + cfqq = cic_to_cfqq(cic, 1); > > > > + if (!cfqq) > > > > + goto out_unlock; > > > > + > > > > + if (cfqd->active_queue != cfqq) > > > > + goto out_unlock; > > > > + > > > > + cfq_log_cfqq(cfqd, cfqq, "yielding queue"); > > > > + > > > > + __cfq_slice_expired(cfqd, cfqq, 1); > > > > + __blk_run_queue(cfqd->queue); > > > > > > I think it would be good if we also check that cfqq is empty or not. If cfqq > > > is not empty we don't want to give up slice? But this is a minor point. At > > > least in case of fsync, we seem to be waiting for all the iozone request > > > to finish and then calling yield. So cfqq should be empty at this point of > > > time. > > > > That's a bit of a problem. Say the queue has one request left, it'll > > service that and the start idling. But we already got this hint that we > > should yield, but as it happened before we reached the idling state, > > we'll not yield. Perhaps it would be better to mark the cfqq as yielding > > if it isn't ready to yield just yet. > > That makes sense. So another cfqq flag in place. As soon as all the > request from cfqq are dispatched, yield the slice instead of idling. And > clear the flag when slice is expiring. Precisely. The next question would be how to control the yielding. In this particular case, you want to be yielding to a specific cfqq. IOW, you essentially want to pass your slide on to that queue. The way the above is implemented, you could easily just switch to another unrelated queue. And if that is done, fairness is skewed without helping the yielding process at all (which was the intention). -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Moyer on 8 Apr 2010 10:30 Jens Axboe <jens.axboe(a)oracle.com> writes: > Precisely. The next question would be how to control the yielding. In > this particular case, you want to be yielding to a specific cfqq. IOW, > you essentially want to pass your slide on to that queue. The way the > above is implemented, you could easily just switch to another unrelated > queue. And if that is done, fairness is skewed without helping the > yielding process at all (which was the intention). Well, that's true in part. Prior to this patch, the process would idle, keeping all other cfq_queues on the system from making progress. With this patch, at least *somebody* else makes progress, getting you closer to running the journal thread that you're blocked on. Ideally, you'd want the thread you're waiting on to get disk time next, sure. You would have to pass the process information down to the I/O scheduler for that, and I'm not sure that the file system code knows which process to hand off to. Does it? Do we really want to go down this road at all? I'm not convinced. Cheers, Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Vivek Goyal on 8 Apr 2010 10:40 On Thu, Apr 08, 2010 at 10:25:50AM -0400, Jeff Moyer wrote: > Vivek Goyal <vgoyal(a)redhat.com> writes: > > > On Thu, Apr 08, 2010 at 10:03:24AM -0400, Jeff Moyer wrote: > >> Which actually brings up the question of whether this needs some > >> knowledge of whether the journal is on the same device as the file > >> system! In such a case, we need not yield. I think I'll stick my head > >> in the sand for this one. ;-) > > > > Jeff even if journal is not on same device, what harm yielding could do? > > Anyway there is no IO on that queue and we are idling. Only side affect is > > that yielding process could lose a bit if after fsync it immediately submits > > more IO. Because this process has yielded it slice, it is back in the queue > > instead of doing more IO in the current slice immediately. > > What happens if the journal is on a super fast device, and finishes up > very quickly allowing our process to initiate more I/O within the idle > window? > You lose. :-) But at the same time if journalling devices is not fast enough, and you if can't submit next IO in idling window, then you are unnecessarily keeping the disk idle and preventing others from making progress. Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jeff Moyer on 8 Apr 2010 15:20 Jens Axboe <jens.axboe(a)oracle.com> writes: >> @@ -2065,7 +2065,7 @@ static void cfq_choose_cfqg(struct cfq_data *cfqd) >> cfqd->serving_group = cfqg; >> >> /* Restore the workload type data */ >> - if (cfqg->saved_workload_slice) { >> + if (cfqg && cfqg->saved_workload_slice) { >> cfqd->workload_expires = jiffies + cfqg->saved_workload_slice; >> cfqd->serving_type = cfqg->saved_workload; >> cfqd->serving_prio = cfqg->saved_serving_prio; > > Unrelated change? Probably not needed for this incarnation of the patch, though previous iterations would Oops on boot w/o this. If you look through all of the code in this code path, cfqg == NULL seems to be handled, so it's probably safe to take this. I'll pull it out of this patch, though. >> +static void cfq_yield(struct request_queue *q) >> +{ >> + struct cfq_data *cfqd = q->elevator->elevator_data; >> + struct cfq_io_context *cic; >> + struct cfq_queue *cfqq; >> + unsigned long flags; >> + >> + cic = cfq_cic_lookup(cfqd, current->io_context); >> + if (!cic) >> + return; >> + >> + spin_lock_irqsave(q->queue_lock, flags); > > spin_lock_irq() is sufficient here. OK, thanks! Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jens Axboe on 8 Apr 2010 15:30
On Thu, Apr 08 2010, Jeff Moyer wrote: > Jens Axboe <jens.axboe(a)oracle.com> writes: > > > Precisely. The next question would be how to control the yielding. In > > this particular case, you want to be yielding to a specific cfqq. IOW, > > you essentially want to pass your slide on to that queue. The way the > > above is implemented, you could easily just switch to another unrelated > > queue. And if that is done, fairness is skewed without helping the > > yielding process at all (which was the intention). > > Well, that's true in part. Prior to this patch, the process would idle, > keeping all other cfq_queues on the system from making progress. With > this patch, at least *somebody* else makes progress, getting you closer > to running the journal thread that you're blocked on. Ideally, you'd > want the thread you're waiting on to get disk time next, sure. You > would have to pass the process information down to the I/O scheduler for > that, and I'm not sure that the file system code knows which process to > hand off to. Does it? > > Do we really want to go down this road at all? I'm not convinced. Don't get me wrong, neither am I. I'm just thinking out loud and pondering. As a general mechanism, yield to a specific cfqq is going to be tricky and doing a generic yield to signal that _someone_ else must make progress before we can is better than nothing. Continuing that train of thought, I don't think we'll ever need full 'yield to X' functionality where 'X' is a really specific target. But for this fsync case, we know at least what we are yielding to and it seems like a shame to throw away that information. So you could include a hint of what to yield to, which cfq could factor in. Dunno, I need to think a bit about the cleanliness of such an approach. We can definitely use your patch as a starting point. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |