diff mbox series

[2/4] io_uring: return error pointer from io_mem_alloc()

Message ID 20230513141643.1037620-3-axboe@kernel.dk (mailing list archive)
State New
Headers show
Series Support for mapping SQ/CQ rings into huge page | expand

Commit Message

Jens Axboe May 13, 2023, 2:16 p.m. UTC 
In preparation for having more than one time of ring allocator, make the
existing one return valid/error-pointer rather than just NULL.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 io_uring/io_uring.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Dmitry Kadashev May 14, 2023, 2:54 a.m. UTC | #1 
Hi Jens,

On Sat, May 13, 2023 at 9:19 PM Jens Axboe <axboe@kernel.dk> wrote:
>
> In preparation for having more than one time of ring allocator, make the
> existing one return valid/error-pointer rather than just NULL.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  io_uring/io_uring.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 3695c5e6fbf0..6266a870c89f 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -2712,8 +2712,12 @@ static void io_mem_free(void *ptr)
>  static void *io_mem_alloc(size_t size)
>  {
>         gfp_t gfp = GFP_KERNEL_ACCOUNT | __GFP_ZERO | __GFP_NOWARN | __GFP_COMP;
> +       void *ret;
>
> -       return (void *) __get_free_pages(gfp, get_order(size));
> +       ret = (void *) __get_free_pages(gfp, get_order(size));
> +       if (ret)
> +               return ret;
> +       return ERR_PTR(-ENOMEM);
>  }
>
>  static unsigned long rings_size(struct io_ring_ctx *ctx, unsigned int sq_entries,
> @@ -3673,6 +3677,7 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>  {
>         struct io_rings *rings;
>         size_t size, sq_array_offset;
> +       void *ptr;
>
>         /* make sure these are sane, as we already accounted them */
>         ctx->sq_entries = p->sq_entries;
> @@ -3683,8 +3688,8 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>                 return -EOVERFLOW;
>
>         rings = io_mem_alloc(size);
> -       if (!rings)
> -               return -ENOMEM;
> +       if (IS_ERR(rings))
> +               return PTR_ERR(rings);
>
>         ctx->rings = rings;
>         ctx->sq_array = (u32 *)((char *)rings + sq_array_offset);
> @@ -3703,13 +3708,14 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>                 return -EOVERFLOW;
>         }
>
> -       ctx->sq_sqes = io_mem_alloc(size);
> -       if (!ctx->sq_sqes) {
> +       ptr = io_mem_alloc(size);
> +       if (IS_ERR(ptr)) {
>                 io_mem_free(ctx->rings);
>                 ctx->rings = NULL;
> -               return -ENOMEM;
> +               return PTR_ERR(ptr);
>         }
>
> +       ctx->sq_sqes = io_mem_alloc(size);

Should be 'ptr' rather than 'io_mem_alloc(size)' here.
Jens Axboe May 14, 2023, 2:59 a.m. UTC | #2 
On 5/13/23 8:54?PM, Dmitry Kadashev wrote:
> Hi Jens,
> 
> On Sat, May 13, 2023 at 9:19?PM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> In preparation for having more than one time of ring allocator, make the
>> existing one return valid/error-pointer rather than just NULL.
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>> ---
>>  io_uring/io_uring.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 3695c5e6fbf0..6266a870c89f 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -2712,8 +2712,12 @@ static void io_mem_free(void *ptr)
>>  static void *io_mem_alloc(size_t size)
>>  {
>>         gfp_t gfp = GFP_KERNEL_ACCOUNT | __GFP_ZERO | __GFP_NOWARN | __GFP_COMP;
>> +       void *ret;
>>
>> -       return (void *) __get_free_pages(gfp, get_order(size));
>> +       ret = (void *) __get_free_pages(gfp, get_order(size));
>> +       if (ret)
>> +               return ret;
>> +       return ERR_PTR(-ENOMEM);
>>  }
>>
>>  static unsigned long rings_size(struct io_ring_ctx *ctx, unsigned int sq_entries,
>> @@ -3673,6 +3677,7 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>>  {
>>         struct io_rings *rings;
>>         size_t size, sq_array_offset;
>> +       void *ptr;
>>
>>         /* make sure these are sane, as we already accounted them */
>>         ctx->sq_entries = p->sq_entries;
>> @@ -3683,8 +3688,8 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>>                 return -EOVERFLOW;
>>
>>         rings = io_mem_alloc(size);
>> -       if (!rings)
>> -               return -ENOMEM;
>> +       if (IS_ERR(rings))
>> +               return PTR_ERR(rings);
>>
>>         ctx->rings = rings;
>>         ctx->sq_array = (u32 *)((char *)rings + sq_array_offset);
>> @@ -3703,13 +3708,14 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>>                 return -EOVERFLOW;
>>         }
>>
>> -       ctx->sq_sqes = io_mem_alloc(size);
>> -       if (!ctx->sq_sqes) {
>> +       ptr = io_mem_alloc(size);
>> +       if (IS_ERR(ptr)) {
>>                 io_mem_free(ctx->rings);
>>                 ctx->rings = NULL;
>> -               return -ENOMEM;
>> +               return PTR_ERR(ptr);
>>         }
>>
>> +       ctx->sq_sqes = io_mem_alloc(size);
> 
> Should be 'ptr' rather than 'io_mem_alloc(size)' here.

Indeed, good catch. Patch 4 does correct that so the final result is
correct, must've happened during a split rebase a while back. I'll fix
up patch 2 and 4 so that it's correct after patch 2 as well.
diff mbox series

Patch

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 3695c5e6fbf0..6266a870c89f 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2712,8 +2712,12 @@  static void io_mem_free(void *ptr)
 static void *io_mem_alloc(size_t size)
 {
 	gfp_t gfp = GFP_KERNEL_ACCOUNT | __GFP_ZERO | __GFP_NOWARN | __GFP_COMP;
+	void *ret;
 
-	return (void *) __get_free_pages(gfp, get_order(size));
+	ret = (void *) __get_free_pages(gfp, get_order(size));
+	if (ret)
+		return ret;
+	return ERR_PTR(-ENOMEM);
 }
 
 static unsigned long rings_size(struct io_ring_ctx *ctx, unsigned int sq_entries,
@@ -3673,6 +3677,7 @@  static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
 {
 	struct io_rings *rings;
 	size_t size, sq_array_offset;
+	void *ptr;
 
 	/* make sure these are sane, as we already accounted them */
 	ctx->sq_entries = p->sq_entries;
@@ -3683,8 +3688,8 @@  static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
 		return -EOVERFLOW;
 
 	rings = io_mem_alloc(size);
-	if (!rings)
-		return -ENOMEM;
+	if (IS_ERR(rings))
+		return PTR_ERR(rings);
 
 	ctx->rings = rings;
 	ctx->sq_array = (u32 *)((char *)rings + sq_array_offset);
@@ -3703,13 +3708,14 @@  static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
 		return -EOVERFLOW;
 	}
 
-	ctx->sq_sqes = io_mem_alloc(size);
-	if (!ctx->sq_sqes) {
+	ptr = io_mem_alloc(size);
+	if (IS_ERR(ptr)) {
 		io_mem_free(ctx->rings);
 		ctx->rings = NULL;
-		return -ENOMEM;
+		return PTR_ERR(ptr);
 	}
 
+	ctx->sq_sqes = io_mem_alloc(size);
 	return 0;
 }