diff mbox series

[09/26] aio: only use blk plugs for > 2 depth submissions

Message ID 20181204233729.26776-10-axboe@kernel.dk (mailing list archive)
State New, archived
Headers show
Series [01/26] fs: add an iopoll method to struct file_operations | expand

Commit Message

Jens Axboe Dec. 4, 2018, 11:37 p.m. UTC 
Plugging is meant to optimize submission of a string of IOs, if we don't
have more than 2 being submitted, don't bother setting up a plug.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 fs/aio.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Comments

Jeff Moyer Dec. 6, 2018, 7:29 p.m. UTC | #1 
Jens Axboe <axboe@kernel.dk> writes:

> Plugging is meant to optimize submission of a string of IOs, if we don't
> have more than 2 being submitted, don't bother setting up a plug.

Is there really that much overhead in blk_{start|finish}_plug?
-Jeff

>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  fs/aio.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/fs/aio.c b/fs/aio.c
> index 522c04864d82..ed6c3914477a 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -69,6 +69,12 @@ struct aio_ring {
>  	struct io_event		io_events[0];
>  }; /* 128 bytes + ring size */
>  
> +/*
> + * Plugging is meant to work with larger batches of IOs. If we don't
> + * have more than the below, then don't bother setting up a plug.
> + */
> +#define AIO_PLUG_THRESHOLD	2
> +
>  #define AIO_RING_PAGES	8
>  
>  struct kioctx_table {
> @@ -1919,7 +1925,8 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr,
>  	if (nr > ctx->nr_events)
>  		nr = ctx->nr_events;
>  
> -	blk_start_plug(&plug);
> +	if (nr > AIO_PLUG_THRESHOLD)
> +		blk_start_plug(&plug);
>  	for (i = 0; i < nr; i++) {
>  		struct iocb __user *user_iocb;
>  
> @@ -1932,7 +1939,8 @@ SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr,
>  		if (ret)
>  			break;
>  	}
> -	blk_finish_plug(&plug);
> +	if (nr > AIO_PLUG_THRESHOLD)
> +		blk_finish_plug(&plug);
>  
>  	percpu_ref_put(&ctx->users);
>  	return i ? i : ret;
> @@ -1959,7 +1967,8 @@ COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
>  	if (nr > ctx->nr_events)
>  		nr = ctx->nr_events;
>  
> -	blk_start_plug(&plug);
> +	if (nr > AIO_PLUG_THRESHOLD)
> +		blk_start_plug(&plug);
>  	for (i = 0; i < nr; i++) {
>  		compat_uptr_t user_iocb;
>  
> @@ -1972,7 +1981,8 @@ COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
>  		if (ret)
>  			break;
>  	}
> -	blk_finish_plug(&plug);
> +	if (nr > AIO_PLUG_THRESHOLD)
> +		blk_finish_plug(&plug);
>  
>  	percpu_ref_put(&ctx->users);
>  	return i ? i : ret;
Jens Axboe Dec. 6, 2018, 7:31 p.m. UTC | #2 
On 12/6/18 12:29 PM, Jeff Moyer wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> Plugging is meant to optimize submission of a string of IOs, if we don't
>> have more than 2 being submitted, don't bother setting up a plug.
> 
> Is there really that much overhead in blk_{start|finish}_plug?

It's not just the overhead of the functions themselves, it's the
pointless work we end up doing for no reason at all. Plugging is, by
defintion, meant to help batch things up, ammortizing the cost of each
individual IO if you have a batch of them. If you don't have a batch,
then there's no point. Normally callers of blk_start_plug() don't
necessarily know how much IO is coming, but in this case we clearly do.
diff mbox series

Patch

diff --git a/fs/aio.c b/fs/aio.c
index 522c04864d82..ed6c3914477a 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -69,6 +69,12 @@  struct aio_ring {
 	struct io_event		io_events[0];
 }; /* 128 bytes + ring size */
 
+/*
+ * Plugging is meant to work with larger batches of IOs. If we don't
+ * have more than the below, then don't bother setting up a plug.
+ */
+#define AIO_PLUG_THRESHOLD	2
+
 #define AIO_RING_PAGES	8
 
 struct kioctx_table {
@@ -1919,7 +1925,8 @@  SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr,
 	if (nr > ctx->nr_events)
 		nr = ctx->nr_events;
 
-	blk_start_plug(&plug);
+	if (nr > AIO_PLUG_THRESHOLD)
+		blk_start_plug(&plug);
 	for (i = 0; i < nr; i++) {
 		struct iocb __user *user_iocb;
 
@@ -1932,7 +1939,8 @@  SYSCALL_DEFINE3(io_submit, aio_context_t, ctx_id, long, nr,
 		if (ret)
 			break;
 	}
-	blk_finish_plug(&plug);
+	if (nr > AIO_PLUG_THRESHOLD)
+		blk_finish_plug(&plug);
 
 	percpu_ref_put(&ctx->users);
 	return i ? i : ret;
@@ -1959,7 +1967,8 @@  COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
 	if (nr > ctx->nr_events)
 		nr = ctx->nr_events;
 
-	blk_start_plug(&plug);
+	if (nr > AIO_PLUG_THRESHOLD)
+		blk_start_plug(&plug);
 	for (i = 0; i < nr; i++) {
 		compat_uptr_t user_iocb;
 
@@ -1972,7 +1981,8 @@  COMPAT_SYSCALL_DEFINE3(io_submit, compat_aio_context_t, ctx_id,
 		if (ret)
 			break;
 	}
-	blk_finish_plug(&plug);
+	if (nr > AIO_PLUG_THRESHOLD)
+		blk_finish_plug(&plug);
 
 	percpu_ref_put(&ctx->users);
 	return i ? i : ret;