AIO: Don't plug the I/O queue in do_io_submit()

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

Quoted 4 lines · Dave Kleikamp

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Quoted 2 lines

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

I'm guessing I know the answer to this, but what workload were you testing, and can you provide more concrete evidence than "latency greatly increased?"

Also, have you tested the effects this has when using traditional storage for whatever your workload is?

I don't feel
comfortable essentially reverting a performance patch without knowing the entire picture. I will certainly do some testing on my end, too.

Cheers,
Jeff

Quoted 6 lines · Jeff Moyer

Quoted 4 lines · Dave Kleikamp

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Quoted 6 lines

Quoted 2 lines

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

I'm guessing I know the answer to this, but what workload were you testing, and can you provide more concrete evidence than "latency greatly increased?"

It is a piece of a larger industry-standard benchmark and you're probably guessing correctly. The "greatly increased" latency was actually slightly higher the improvement I get with this patch. So the patch brought the latency nearly down to where it was before.

I will try a microbenchmark to see if I get similar behavior, but I
wanted to throw this out here to get input.

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

Signed-off-by Dave Kleikamp <[email protected]>

Cc [email protected]

Cc Chris Mason <[email protected]>

Cc Jens Axboe <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jeff Moyer <[email protected]>

fs/aio.c +0 -3

index 78c514c..d131a2c 100644
@@ -1696,7 +1696,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
        struct kioctx *ctx;
        long ret = 0;
        int i = 0;
-       struct blk_plug plug;
        struct kiocb_batch batch;
 
        if (unlikely(nr < 0))

@@ -1716,8 +1715,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
 
        kiocb_batch_init(&batch, nr);
 
-       blk_start_plug(&plug);
-
        /*
         * AKPM: should this return a partial result if some of the IOs were
         * successfully submitted?

2011/12/14 Dave Kleikamp <[email protected]>:

Quoted 41 lines

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

Signed-off-by Dave Kleikamp <[email protected]>

Cc [email protected]

Cc Chris Mason <[email protected]>

Cc Jens Axboe <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jeff Moyer <[email protected]>

fs/aio.c +0 -4

index 78c514c..d131a2c 100644
@@ -1696,7 +1696,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
       struct kioctx *ctx;
       long ret = 0;
       int i = 0;
-       struct blk_plug plug;
       struct kiocb_batch batch;
 
       if (unlikely(nr < 0))

@@ -1716,8 +1715,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
 
       kiocb_batch_init(&batch, nr);
 
-       blk_start_plug(&plug);
-
       /*
        * AKPM: should this return a partial result if some of the IOs were
        * successfully submitted?

@@ -1740,7 +1737,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
               if (ret)
                       break;
       }
-       blk_finish_plug(&plug);
 

fs/aio.c long do_io_submit(aio_context_t ctx_id, long nr, · line 1742

Quoted 2 lines

fs/aio.c +0 -0

       kiocb_batch_free(&batch);
       put_ioctx(ctx);

can you explain why this can help? Note, in 3.1 kernel we now force flush plug list if the list is too long, which will remove a lot of latency.

Quoted 42 lines · Shaohua Li

2011/12/14 Dave Kleikamp <[email protected]>:

Quoted 41 lines

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

Signed-off-by Dave Kleikamp <[email protected]>

Cc [email protected]

Cc Chris Mason <[email protected]>

Cc Jens Axboe <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jeff Moyer <[email protected]>

fs/aio.c +0 -4

index 78c514c..d131a2c 100644
@@ -1696,7 +1696,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
       struct kioctx *ctx;
       long ret = 0;
       int i = 0;
-       struct blk_plug plug;
       struct kiocb_batch batch;
 
       if (unlikely(nr < 0))

@@ -1716,8 +1715,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
 
       kiocb_batch_init(&batch, nr);
 
-       blk_start_plug(&plug);
-
       /*
        * AKPM: should this return a partial result if some of the IOs were
        * successfully submitted?

@@ -1740,7 +1737,6 @@ long do_io_submit(aio_context_t ctx_id, long nr,
               if (ret)
                       break;
       }
-       blk_finish_plug(&plug);
 

Quoted 4 lines

fs/aio.c long do_io_submit(aio_context_t ctx_id, long nr, · line 1742

Quoted 2 lines

fs/aio.c +0 -0

       kiocb_batch_free(&batch);
       put_ioctx(ctx);

can you explain why this can help? Note, in 3.1 kernel we now force flush plug list if the list is too long, which will remove a lot of latency.

I think that would indeed be an interesting addition to test on top of the 3.0 kernel being used.

This is a bit of a sticky situation. We want the plugging and merging on rotational storage, and on SSDs we want the batch addition to the queue to avoid hammering on the queue lock. At this level, we have no idea. But we don't want to introduce longer latencies. So the question is, are these latencies due to long queues (and hence would be helped with the auto-replug on 3.1 and newer), or are they due to the submissions running for too long. If the latter, then we can either look into reducing the time spent between submitting the individual pieces. Or at least not holding up too long.

Quoted 14 lines · Dave Kleikamp

Quoted 6 lines · Jeff Moyer

Quoted 4 lines · Dave Kleikamp

Asynchronous I/O latency to a solid-state disk greatly increased between the 2.6.32 and 3.0 kernels. By removing the plug from do_io_submit(), we observed a 34% improvement in the I/O latency.

Quoted 6 lines

Quoted 2 lines

Unfortunately, at this level, we don't know if the request is to a rotating disk or not.

I'm guessing I know the answer to this, but what workload were you testing, and can you provide more concrete evidence than "latency greatly increased?"

Quoted 7 lines

It is a piece of a larger industry-standard benchmark and you're probably guessing correctly. The "greatly increased" latency was actually slightly higher the improvement I get with this patch. So the patch brought the latency nearly down to where it was before.

I will try a microbenchmark to see if I get similar behavior, but I
wanted to throw this out here to get input.

The better IO latency did bump the overall benchmark score by 3%, and it did end up bringing our latencies on par with solaris runs on similar hardware.

We didn't find this one through exhaustive tracing...instead we used a more traditional approach involving a list of Jens' commits and a dart board. So, we don't have a lot of data yet on exactly why the plug is hurting.

But, I'm starting to wonder if the plug makes sense here at all. We're queueing up IO in the main submit loop, and the aio submit might be spanning any number of devices on a large variety of filesystems. The actual direct IO call may be pretty expensive.

My guess for why this helps is contention on the aio context lock between the submission code and the end_io softirq code. We bash on that lock a number of times during the IO submit, and the whole time we're holding on to our list of plugged IOs instead of giving the hardware the chance to process them.