cfq-iosched: fixing RQ_NOIDLE handling.
in [Kernel]
|
Prev: cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD
Next: [PATCH 2/2] cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD
From: Vivek Goyal on 9 Jul 2010 09:30 On Fri, Jul 09, 2010 at 12:33:36PM +0200, Corrado Zoccolo wrote: > On Wed, Jul 7, 2010 at 10:06 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: > > Corrado Zoccolo <czoccolo(a)gmail.com> writes: > > > >> On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: > >>> Corrado Zoccolo <czoccolo(a)gmail.com> writes: > >>> > >>>> Hi Jens, > >>>> patch 8e55063 "cfq-iosched: fix corner cases in idling logic", is > >>>> suspected for some regressions on high end hardware. > >>>> The two patches from this series: > >>>> - [PATCH 1/2] cfq-iosched: fix tree-wide handling of rq_noidle > >>>> - [PATCH 2/2] cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD > >>>> fix two issues that I have identified, related to how RQ_NOIDLE is > >>>> used by the upper layers. > >>>> First patch makes sure that a RQ_NOIDLE coming after a sequence of > >>>> possibly idling requests from the same queue on the no-idle tree will > >>>> clear the noidle_tree_requires_idle flag. > >>>> Second patch enables RQ_NOIDLE for queues in the idling tree, > >>>> restoring the behaviour pre-8e55063 patch. > >>> > >>> Hi, Corrado, > >>> > >>> I ran your kernel through my tests. �Here are the results, up against > >>> vanilla, deadline, and the blk_yield patch set: > >>> > >> Hi Jeff, > >> can you also add cfq with 8e55063 reverted to the testing mix? > > > > Sure, the results now look like this: > > > > � � � � � � � � just � �just > > � � � � � � � �fs_mark �fio � � � �mixed > > -------------------------------+-------------- > > deadline � � � �529.44 � 151.4 | 450.0 � �78.2 > > vanilla cfq � � 107.88 � 164.4 | � 6.6 � 137.2 > > blk_yield cfq � 530.82 � 158.7 | 113.2 � �78.6 > > corrado cfq � � 110.16 � 220.6 | � 7.0 � 159.8 > > 8e55063 revert �559.66 � 198.9 | �16.1 � 153.3 > > > > I had accidentally run your patch set (corrado cfq) on ext3, so the > > numbers were a bit off (everything else was run against ext4). �The > > corrected numbers above reflect the performance on ext4, which is much > > better for the sequential reader, but still not great for the fs_mark > > run. �Reverting 8e55063 definitely gets us into better shape. �However, > > if we care about the mixed workload, then it won't be enough. > > Wondering why deadline performs so well in the fs_mark workload. Is it > because it doesn't distinguish between sync and async writes? > Maybe we can achieve something similar by putting all sync writes > (that are marked as REQ_NOIDLE) in the noidle tree? This, coupled with > making jbd(2) perform sync writes, should make the yield automatic, > since they all live in the same tree for which we don't idle between > queues, and should be able to provide fairness compared to a > sequential reader (that lives in the other tree). > This makes sense conceptually at least at CFQ level. By putting OSYNC/fsync on sync-noidle tree we will not be able to take advantage of sequential nature of queue but christoph mentioned that all sequential writes in general should be lumped together and then sent down to CFQ instead of issuing small writes after some delay. So this probably is not an issue. What I am not sure about is impact of switching jbd thread writes from async to sync (WRITE ---> WRITE_SYNC). Especially if somebody is journalling the data also (data=journal). But it is definitely worth trying because then we don't have to idle on individual queues of WRITE_SYNC as well as fsync performance issue should also be solved. Thanks 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 9 Jul 2010 10:10 Corrado Zoccolo <czoccolo(a)gmail.com> writes: > On Wed, Jul 7, 2010 at 10:06 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: >> Corrado Zoccolo <czoccolo(a)gmail.com> writes: >> >>> On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: >>>> Corrado Zoccolo <czoccolo(a)gmail.com> writes: >>>> >>>>> Hi Jens, >>>>> patch 8e55063 "cfq-iosched: fix corner cases in idling logic", is >>>>> suspected for some regressions on high end hardware. >>>>> The two patches from this series: >>>>> - [PATCH 1/2] cfq-iosched: fix tree-wide handling of rq_noidle >>>>> - [PATCH 2/2] cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD >>>>> fix two issues that I have identified, related to how RQ_NOIDLE is >>>>> used by the upper layers. >>>>> First patch makes sure that a RQ_NOIDLE coming after a sequence of >>>>> possibly idling requests from the same queue on the no-idle tree will >>>>> clear the noidle_tree_requires_idle flag. >>>>> Second patch enables RQ_NOIDLE for queues in the idling tree, >>>>> restoring the behaviour pre-8e55063 patch. >>>> >>>> Hi, Corrado, >>>> >>>> I ran your kernel through my tests. Here are the results, up against >>>> vanilla, deadline, and the blk_yield patch set: >>>> >>> Hi Jeff, >>> can you also add cfq with 8e55063 reverted to the testing mix? >> >> Sure, the results now look like this: >> >> just just >> fs_mark fio mixed >> -------------------------------+-------------- >> deadline 529.44 151.4 | 450.0 78.2 >> vanilla cfq 107.88 164.4 | 6.6 137.2 >> blk_yield cfq 530.82 158.7 | 113.2 78.6 >> corrado cfq 110.16 220.6 | 7.0 159.8 >> 8e55063 revert 559.66 198.9 | 16.1 153.3 >> >> I had accidentally run your patch set (corrado cfq) on ext3, so the >> numbers were a bit off (everything else was run against ext4). The >> corrected numbers above reflect the performance on ext4, which is much >> better for the sequential reader, but still not great for the fs_mark >> run. Reverting 8e55063 definitely gets us into better shape. However, >> if we care about the mixed workload, then it won't be enough. > > Wondering why deadline performs so well in the fs_mark workload. Is it > because it doesn't distinguish between sync and async writes? It performs well because it doesn't do any idling. > Maybe we can achieve something similar by putting all sync writes > (that are marked as REQ_NOIDLE) in the noidle tree? This, coupled with > making jbd(2) perform sync writes, should make the yield automatic, > since they all live in the same tree for which we don't idle between > queues, and should be able to provide fairness compared to a > sequential reader (that lives in the other tree). > > Can you test the attached patch, where I also added your changes to > make jbd(2) to perform sync writes? I'm not sure what kernel you generated that patch against. I'm working with 2.6.35-rc3 or later, and your patch does not apply there. 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: Corrado Zoccolo on 9 Jul 2010 15:50 On Fri, Jul 9, 2010 at 4:07 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: > Corrado Zoccolo <czoccolo(a)gmail.com> writes: > >> On Wed, Jul 7, 2010 at 10:06 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: >>> Corrado Zoccolo <czoccolo(a)gmail.com> writes: >>> >>>> On Wed, Jul 7, 2010 at 7:03 PM, Jeff Moyer <jmoyer(a)redhat.com> wrote: >>>>> Corrado Zoccolo <czoccolo(a)gmail.com> writes: >>>>> >>>>>> Hi Jens, >>>>>> patch 8e55063 "cfq-iosched: fix corner cases in idling logic", is >>>>>> suspected for some regressions on high end hardware. >>>>>> The two patches from this series: >>>>>> - [PATCH 1/2] cfq-iosched: fix tree-wide handling of rq_noidle >>>>>> - [PATCH 2/2] cfq-iosched: RQ_NOIDLE enabled for SYNC_WORKLOAD >>>>>> fix two issues that I have identified, related to how RQ_NOIDLE is >>>>>> used by the upper layers. >>>>>> First patch makes sure that a RQ_NOIDLE coming after a sequence of >>>>>> possibly idling requests from the same queue on the no-idle tree will >>>>>> clear the noidle_tree_requires_idle flag. >>>>>> Second patch enables RQ_NOIDLE for queues in the idling tree, >>>>>> restoring the behaviour pre-8e55063 patch. >>>>> >>>>> Hi, Corrado, >>>>> >>>>> I ran your kernel through my tests. Here are the results, up against >>>>> vanilla, deadline, and the blk_yield patch set: >>>>> >>>> Hi Jeff, >>>> can you also add cfq with 8e55063 reverted to the testing mix? >>> >>> Sure, the results now look like this: >>> >>> just just >>> fs_mark fio mixed >>> -------------------------------+-------------- >>> deadline 529.44 151.4 | 450.0 78.2 >>> vanilla cfq 107.88 164.4 | 6.6 137.2 >>> blk_yield cfq 530.82 158.7 | 113.2 78.6 >>> corrado cfq 110.16 220.6 | 7.0 159.8 >>> 8e55063 revert 559.66 198.9 | 16.1 153.3 >>> >>> I had accidentally run your patch set (corrado cfq) on ext3, so the >>> numbers were a bit off (everything else was run against ext4). The >>> corrected numbers above reflect the performance on ext4, which is much >>> better for the sequential reader, but still not great for the fs_mark >>> run. Reverting 8e55063 definitely gets us into better shape. However, >>> if we care about the mixed workload, then it won't be enough. >> >> Wondering why deadline performs so well in the fs_mark workload. Is it >> because it doesn't distinguish between sync and async writes? > > It performs well because it doesn't do any idling. > >> Maybe we can achieve something similar by putting all sync writes >> (that are marked as REQ_NOIDLE) in the noidle tree? This, coupled with >> making jbd(2) perform sync writes, should make the yield automatic, >> since they all live in the same tree for which we don't idle between >> queues, and should be able to provide fairness compared to a >> sequential reader (that lives in the other tree). >> >> Can you test the attached patch, where I also added your changes to >> make jbd(2) to perform sync writes? > > I'm not sure what kernel you generated that patch against. I'm working > with 2.6.35-rc3 or later, and your patch does not apply there. It's Jens' block/for-2.6.36 tree. > > Cheers, > Jeff > -- __________________________________________________________________________ dott. Corrado Zoccolo mailto:czoccolo(a)gmail.com PhD - Department of Computer Science - University of Pisa, Italy -------------------------------------------------------------------------- The self-confidence of a warrior is not the self-confidence of the average man. The average man seeks certainty in the eyes of the onlooker and calls that self-confidence. The warrior seeks impeccability in his own eyes and calls that humbleness. Tales of Power - C. Castaneda -- 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 9 Jul 2010 16:50 Corrado Zoccolo <czoccolo(a)gmail.com> writes: >> I'm not sure what kernel you generated that patch against. I'm working >> with 2.6.35-rc3 or later, and your patch does not apply there. > It's Jens' block/for-2.6.36 tree. OK. I'll get back to you on this next week. My storage is kind of broken right now. :( -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: Jeff Moyer on 13 Jul 2010 15:40
Corrado Zoccolo <czoccolo(a)gmail.com> writes: > Can you test the attached patch, where I also added your changes to > make jbd(2) to perform sync writes? I got new storage, so I have new numbers. I only re-ran deadline and vanilla cfq for the fs_mark only test. The average of 10 runs comes out like so: deadline: 571.98 vanilla cfq: 107.42 patched cfq: 460.9 Mixed workload results with your suggested patch: fs_mark: 15.65 files/sec fio: 132.5 MB/s So, again, not looking great for the mixed workload, but the patch does improve the fs_mark only case. Looking at the blktrace data shows that the jbd2 thread preempts the fs_mark thread at all the right times. The only thing holding throughput back is the whole notion that we need to only dispatch from one queue (even though the storage is capable of serving both the reads and writes simultaneously). I added in the patch that allows the simultaneous dispatch of both reads and writes, and here are the results from that run: fs_mark: 15.975 files/sec fio: 132.4 MB/s So, it looks like that didn't help. The reason this patch doesn't come close to the yield patch in the mixed workload is because the yield patch set allows the fs_mark process to continue to issue I/O. With your patch, the fs_mark process does 64KB of I/O, the jbd2 thread does the journal commit, and then the fio process runs again. Given that the fs_mark process typically only uses a small fraction of its time slice, you end up with an unfair balance. Now, we still have to decide whether that's a problem that needs solving. I tried to gather data from the field, but I've been unable to conclusively say whether an application issues this sort of dependent I/O. As such, I am happy with this patch. If we see that we need something like the blk_yield approach, then I'm happy to resurrect that work. Jens, do you find that an agreeable solution? If so, you can add my signed-off-by and tested-by to the patch that Corrado posted. 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/ |