Messages in this thread | | | Date | Mon, 11 Jan 2010 11:44:33 -0500 | From | Vivek Goyal <> | Subject | Re: [PATCH] cfq-iosched: NCQ SSDs do not need read queue merging |
| |
On Mon, Jan 11, 2010 at 03:53:00PM +0100, Corrado Zoccolo wrote: > On Mon, Jan 11, 2010 at 2:18 PM, Jeff Garzik <jeff@garzik.org> wrote: > > On 01/11/2010 08:13 AM, Jens Axboe wrote: > >> > >> On Mon, Jan 11 2010, Corrado Zoccolo wrote: > >>> > >>> On Mon, Jan 11, 2010 at 12:25 PM, Jeff Garzik<jeff@garzik.org> wrote: > >>>> > >>>> On 01/10/2010 04:04 PM, Corrado Zoccolo wrote: > >>>>> > >>>>> NCQ SSDs' performances are not affected by > >>>>> distance of read requests, so there is no point in having > >>>>> overhead to merge such queues. > >>>>> > >>>>> Non-NCQ SSDs showed regression in some special cases, so > >>>>> they are ruled out by this patch. > >>>>> > >>>>> This patch intentionally doesn't affect writes, so > >>>>> it changes the queued[] field, to be indexed by > >>>>> READ/WRITE instead of SYNC/ASYNC, and only compute proximity > >>>>> for queues with WRITE requests. > >>>>> > >>>>> Signed-off-by: Corrado Zoccolo<czoccolo@gmail.com> > >>>> > >>>> That's not really true. Overhead always increases as the total number > >>>> of > >>>> ATA commands issued increases. > >>> > >>> Jeff Moyer tested the patch on the workload that mostly benefit of > >>> queue merging, and found that > >>> the performance was improved by the patch. > >>> So removing the CPU overhead helps much more than the marginal gain > >>> given by merging on this hardware. > >> > >> It's not always going to be true. On SATA the command overhead is fairly > >> low, but on other hardware that may not be the case. Unless you are CPU > >> bound by your IO device, then merging will always be beneficial. I'm a > >> little behind on emails after my vacation, Jeff what numbers did you > >> generate and on what hardware? > > > > ...and on what workload? "the workload that mostly benefit of queue > > merging" is highly subjective, and likely does not cover most workloads SSDs > > will see in the field. > Hi Jeff, > exactly. > The workloads that benefits from queue merging are the ones in which a > sequential > read is actually splitt, and carried out by different processes in > different I/O context, each > sending requests with strides. This is clearly not the best way of > doing sequential access > (I would happily declare those programs as buggy). > CFQ has code that merges queues in this case. I'm disabling the READ > part for NCQ SSDs, > since, as Jeff measured, the code overhead outweight the gain from > merging (if any).
Hi Corrado,
In Jeff's test case of running read-test2, I am not even sure if any merging between the queues took place or not as on NCQ SSD, we are driving deeper queue depths and unless read-test2 is creating more than 32 threads, there might not be any merging taking place at all.
We also don't have any data/numbers what kind of cpu savings does this patch bring in.
Vivek
> > As you said, most workloads don't benefit from queue merging. On those > ones, the patch > just removes an overhead. > > Thanks, > Corrado > > > Jeff > > > > > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |