[PATCH for-next 0/3] ranged file slot alloc

[PATCH for-next 0/3] ranged file slot alloc

[Pavel Begunkov]

Add helpers and test ranged file slot allocation feature

Pavel Begunkov (3):
  update io_uring.h with file slot alloc ranges
  alloc range helpers
  test range file alloc

 src/include/liburing.h          |   3 +
 src/include/liburing/io_uring.h |  10 ++
 src/register.c                  |  14 +++
 test/file-register.c            | 171 ++++++++++++++++++++++++++++++++
 4 files changed, 198 insertions(+)

-- 
2.36.1

[PATCH for-next 1/3] update io_uring.h with file slot alloc ranges

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/include/liburing/io_uring.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
index 0fd1f98..c01c5a3 100644
--- a/src/include/liburing/io_uring.h
+++ b/src/include/liburing/io_uring.h
@@ -414,6 +414,9 @@ enum {
 	/* sync cancelation API */
 	IORING_REGISTER_SYNC_CANCEL		= 24,
 
+	/* register a range of fixed file slots for automatic slot allocation */
+	IORING_REGISTER_FILE_ALLOC_RANGE	= 25,
+
 	/* this goes last */
 	IORING_REGISTER_LAST
 };
@@ -558,6 +561,13 @@ struct io_uring_getevents_arg {
 	__u64	ts;
 };
 
+struct io_uring_file_index_range {
+	/* [off, off + len) */
+	__u32	off;
+	__u32	len;
+	__u64	resv;
+};
+
 /*
  * accept flags stored in sqe->ioprio
  */
-- 
2.36.1

[PATCH for-next 2/3] alloc range helpers

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/include/liburing.h |  3 +++
 src/register.c         | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index bb2fb87..45b4da0 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -186,6 +186,9 @@ int io_uring_unregister_buf_ring(struct io_uring *ring, int bgid);
 int io_uring_register_sync_cancel(struct io_uring *ring,
 				 struct io_uring_sync_cancel_reg *reg);
 
+int io_uring_register_file_alloc_range(struct io_uring *ring,
+					unsigned off, unsigned len);
+
 /*
  * Helper for the peek/wait single cqe functions. Exported because of that,
  * but probably shouldn't be used directly in an application.
diff --git a/src/register.c b/src/register.c
index f2b1026..ee370d6 100644
--- a/src/register.c
+++ b/src/register.c
@@ -352,3 +352,17 @@ int io_uring_register_sync_cancel(struct io_uring *ring,
 	return ____sys_io_uring_register(ring->ring_fd,
 					 IORING_REGISTER_SYNC_CANCEL, reg, 1);
 }
+
+int io_uring_register_file_alloc_range(struct io_uring *ring,
+					unsigned off, unsigned len)
+{
+	struct io_uring_file_index_range range;
+
+	memset(&range, 0, sizeof(range));
+	range.off = off;
+	range.len = len;
+
+	return ____sys_io_uring_register(ring->ring_fd,
+					 IORING_REGISTER_FILE_ALLOC_RANGE,
+					 &range, 0);
+}
-- 
2.36.1

Re: [PATCH for-next 2/3] alloc range helpers

[Jens Axboe]

On 6/30/22 3:13 AM, Pavel Begunkov wrote:
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  src/include/liburing.h |  3 +++
>  src/register.c         | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/src/include/liburing.h b/src/include/liburing.h
> index bb2fb87..45b4da0 100644
> --- a/src/include/liburing.h
> +++ b/src/include/liburing.h
> @@ -186,6 +186,9 @@ int io_uring_unregister_buf_ring(struct io_uring *ring, int bgid);
>  int io_uring_register_sync_cancel(struct io_uring *ring,
>  				 struct io_uring_sync_cancel_reg *reg);
>  
> +int io_uring_register_file_alloc_range(struct io_uring *ring,
> +					unsigned off, unsigned len);

This should go into liburing.map as well.

-- 
Jens Axboe

[PATCH for-next 3/3] test range file alloc

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 test/file-register.c | 171 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 171 insertions(+)

diff --git a/test/file-register.c b/test/file-register.c
index cd00a90..dc4e685 100644
--- a/test/file-register.c
+++ b/test/file-register.c
@@ -9,6 +9,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <sys/resource.h>
 
 #include "helpers.h"
@@ -830,6 +831,170 @@ static int test_partial_register_fail(void)
 	return 0;
 }
 
+static int file_update_alloc(struct io_uring *ring, int *fd)
+{
+	struct io_uring_sqe *sqe;
+	struct io_uring_cqe *cqe;
+	int ret;
+
+	sqe = io_uring_get_sqe(ring);
+	io_uring_prep_files_update(sqe, fd, 1, IORING_FILE_INDEX_ALLOC);
+
+	ret = io_uring_submit(ring);
+	if (ret != 1) {
+		fprintf(stderr, "%s: got %d, wanted 1\n", __FUNCTION__, ret);
+		return -1;
+	}
+
+	ret = io_uring_wait_cqe(ring, &cqe);
+	if (ret < 0) {
+		fprintf(stderr, "%s: io_uring_wait_cqe=%d\n", __FUNCTION__, ret);
+		return -1;
+	}
+	ret = cqe->res;
+	io_uring_cqe_seen(ring, cqe);
+	return ret;
+}
+
+static int test_out_of_range_file_ranges(struct io_uring *ring)
+{
+	int ret;
+
+	ret = io_uring_register_file_alloc_range(ring, 8, 3);
+	if (ret != -EINVAL) {
+		fprintf(stderr, "overlapping range %i\n", ret);
+		return 1;
+	}
+
+	ret = io_uring_register_file_alloc_range(ring, 10, 1);
+	if (ret != -EINVAL) {
+		fprintf(stderr, "out of range index %i\n", ret);
+		return 1;
+	}
+
+	ret = io_uring_register_file_alloc_range(ring, 7, ~1U);
+	if (ret != -EOVERFLOW) {
+		fprintf(stderr, "overflow %i\n", ret);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int test_overallocating_file_range(struct io_uring *ring, int fds[2])
+{
+	int roff = 7, rlen = 2;
+	int ret, i, fd;
+
+	ret = io_uring_register_file_alloc_range(ring, roff, rlen);
+	if (ret) {
+		fprintf(stderr, "io_uring_register_file_alloc_range %i\n", ret);
+		return 1;
+	}
+
+	for (i = 0; i < rlen; i++) {
+		fd = fds[0];
+		ret = file_update_alloc(ring, &fd);
+		if (ret != 1) {
+			fprintf(stderr, "file_update_alloc\n");
+			return 1;
+		}
+
+		if (fd < roff || fd >= roff + rlen) {
+			fprintf(stderr, "invalid off result %i\n", fd);
+			return 1;
+		}
+	}
+
+	fd = fds[0];
+	ret = file_update_alloc(ring, &fd);
+	if (ret != -ENFILE) {
+		fprintf(stderr, "overallocated %i, off %i\n", ret, fd);
+		return 1;
+	}
+
+	return 0;
+}
+
+static int test_zero_range_alloc(struct io_uring *ring, int fds[2])
+{
+	int ret, fd;
+
+	ret = io_uring_register_file_alloc_range(ring, 7, 0);
+	if (ret) {
+		fprintf(stderr, "io_uring_register_file_alloc_range failed %i\n", ret);
+		return 1;
+	}
+
+	fd = fds[0];
+	ret = file_update_alloc(ring, &fd);
+	if (ret != -ENFILE) {
+		fprintf(stderr, "zero alloc %i\n", ret);
+		return 1;
+	}
+	return 0;
+}
+
+static int test_file_alloc_ranges(void)
+{
+	struct io_uring ring;
+	int ret, pipe_fds[2];
+
+	if (pipe(pipe_fds)) {
+		fprintf(stderr, "pipes\n");
+		return 1;
+	}
+	ret = io_uring_queue_init(8, &ring, 0);
+	if (ret) {
+		fprintf(stderr, "queue_init: %d\n", ret);
+		return 1;
+	}
+
+	ret = io_uring_register_files_sparse(&ring, 10);
+	if (ret == -EINVAL) {
+not_supported:
+		close(pipe_fds[0]);
+		close(pipe_fds[1]);
+		io_uring_queue_exit(&ring);
+		printf("file alloc ranges are not supported, skip\n");
+		return 0;
+	} else if (ret) {
+		fprintf(stderr, "io_uring_register_files_sparse %i\n", ret);
+		return ret;
+	}
+
+	ret = io_uring_register_file_alloc_range(&ring, 0, 1);
+	if (ret) {
+		if (ret == -EINVAL)
+			goto not_supported;
+		fprintf(stderr, "io_uring_register_file_alloc_range %i\n", ret);
+		return 1;
+	}
+
+	ret = test_overallocating_file_range(&ring, pipe_fds);
+	if (ret) {
+		fprintf(stderr, "test_overallocating_file_range() failed\n");
+		return 1;
+	}
+
+	ret = test_out_of_range_file_ranges(&ring);
+	if (ret) {
+		fprintf(stderr, "test_out_of_range_file_ranges() failed\n");
+		return 1;
+	}
+
+	ret = test_zero_range_alloc(&ring, pipe_fds);
+	if (ret) {
+		fprintf(stderr, "test_zero_range_alloc() failed\n");
+		return 1;
+	}
+
+	close(pipe_fds[0]);
+	close(pipe_fds[1]);
+	io_uring_queue_exit(&ring);
+	return 0;
+}
+
 int main(int argc, char *argv[])
 {
 	struct io_uring ring;
@@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
 		return ret;
 	}
 
+	ret = test_file_alloc_ranges();
+	if (ret) {
+		printf("test_partial_register_fail failed\n");
+		return ret;
+	}
+
 	return T_EXIT_PASS;
 }
-- 
2.36.1

Re: [PATCH for-next 3/3] test range file alloc

[Jens Axboe]

On 6/30/22 3:13 AM, Pavel Begunkov wrote:
> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>  		return ret;
>  	}
>  
> +	ret = test_file_alloc_ranges();
> +	if (ret) {
> +		printf("test_partial_register_fail failed\n");
> +		return ret;
> +	}

If you're returning this directly, test_file_alloc_ranges() should use
the proper T_EXIT_foo return codes.

-- 
Jens Axboe

Re: [PATCH for-next 3/3] test range file alloc

[Ammar Faizi]

On 6/30/22 8:09 PM, Jens Axboe wrote:
> On 6/30/22 3:13 AM, Pavel Begunkov wrote:
>> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>>   		return ret;
>>   	}
>>   
>> +	ret = test_file_alloc_ranges();
>> +	if (ret) {
>> +		printf("test_partial_register_fail failed\n");
>> +		return ret;
>> +	}
> 
> If you're returning this directly, test_file_alloc_ranges() should use
> the proper T_EXIT_foo return codes.

Also use `fprintf(stderr, ...)` instead of `printf(...)` for that one.

-- 
Ammar Faizi

Re: [PATCH for-next 3/3] test range file alloc

[Pavel Begunkov]

On 6/30/22 14:09, Jens Axboe wrote:
> On 6/30/22 3:13 AM, Pavel Begunkov wrote:
>> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>>   		return ret;
>>   	}
>>   
>> +	ret = test_file_alloc_ranges();
>> +	if (ret) {
>> +		printf("test_partial_register_fail failed\n");
>> +		return ret;
>> +	}
> 
> If you're returning this directly, test_file_alloc_ranges() should use
> the proper T_EXIT_foo return codes.

Nobody cared enough to "fix" all tests to use those new codes, most
of the cases just return what they've got, but whatever. Same with
stdout vs stderr.

-- 
Pavel Begunkov

Re: [PATCH for-next 3/3] test range file alloc

[Ammar Faizi]

On 6/30/22 9:19 PM, Pavel Begunkov wrote:
> On 6/30/22 14:09, Jens Axboe wrote:
>> On 6/30/22 3:13 AM, Pavel Begunkov wrote:
>>> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>>>           return ret;
>>>       }
>>> +    ret = test_file_alloc_ranges();
>>> +    if (ret) {
>>> +        printf("test_partial_register_fail failed\n");
>>> +        return ret;
>>> +    }
>>
>> If you're returning this directly, test_file_alloc_ranges() should use
>> the proper T_EXIT_foo return codes.
> 
> Nobody cared enough to "fix" all tests to use those new codes, most
> of the cases just return what they've got, but whatever. Same with
> stdout vs stderr.

That error code rule was invented since commit:

    68103b731c34a9f83c181cb33eb424f46f3dcb94 ("Merge branch 'exitcode-protocol' of ....")

    Ref: https://github.com/axboe/liburing/pull/621/files

Thanks to Eli who did it. Eli also fixed all tests. Maybe some are still
missing, but if we find it, better to fix it.

-- 
Ammar Faizi

Re: [PATCH for-next 3/3] test range file alloc

[Pavel Begunkov]

On 6/30/22 15:31, Ammar Faizi wrote:
> On 6/30/22 9:19 PM, Pavel Begunkov wrote:
>> On 6/30/22 14:09, Jens Axboe wrote:
>>> On 6/30/22 3:13 AM, Pavel Begunkov wrote:
>>>> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>>>>           return ret;
>>>>       }
>>>> +    ret = test_file_alloc_ranges();
>>>> +    if (ret) {
>>>> +        printf("test_partial_register_fail failed\n");
>>>> +        return ret;
>>>> +    }
>>>
>>> If you're returning this directly, test_file_alloc_ranges() should use
>>> the proper T_EXIT_foo return codes.
>>
>> Nobody cared enough to "fix" all tests to use those new codes, most
>> of the cases just return what they've got, but whatever. Same with
>> stdout vs stderr.
> 
> That error code rule was invented since commit:
> 
>     68103b731c34a9f83c181cb33eb424f46f3dcb94 ("Merge branch 'exitcode-protocol' of ....")
> 
>     Ref: https://github.com/axboe/liburing/pull/621/files
> 
> Thanks to Eli who did it. Eli also fixed all tests. Maybe some are still
> missing, but if we find it, better to fix it.

Have no idea what you're talking about but I'm having
hard time calling 6 returns out of 21 in this file "all".

-- 
Pavel Begunkov

Re: [PATCH for-next 3/3] test range file alloc

[Eli Schwartz]

On 6/30/22 11:18 AM, Pavel Begunkov wrote:
> On 6/30/22 15:31, Ammar Faizi wrote:
>> On 6/30/22 9:19 PM, Pavel Begunkov wrote:
>>> Nobody cared enough to "fix" all tests to use those new codes, most
>>> of the cases just return what they've got, but whatever. Same with
>>> stdout vs stderr.
>>
>> That error code rule was invented since commit:
>>
>>     68103b731c34a9f83c181cb33eb424f46f3dcb94 ("Merge branch
>> 'exitcode-protocol' of ....")
>>
>>     Ref: https://github.com/axboe/liburing/pull/621/files
>>
>> Thanks to Eli who did it. Eli also fixed all tests. Maybe some are still
>> missing, but if we find it, better to fix it.
> 
> Have no idea what you're talking about but I'm having
> hard time calling 6 returns out of 21 in this file "all".


Hi, I should probably clarify the state of affairs...

I submitted a patch series on github 4 days ago which implements those
new codes. It was merged 2 days ago. This is very new code, so I think
it's not completely 100% fair to say that no one "cared" enough to use it.

As far as the actual changes and their completion go... take a look at
the commit messages in the merged patches, specifically take a look at
commit ed430fbeb33367324a039d9cee0fd504bb91e11a.

"""
tests: migrate some tests to use enum-based exit codes

[...]

A partial migration of existing pass/fail values in test sources is
included.
"""

You can also take a look at Github's equivalent of a cover letter, in
which I mentioned that I haven't ported everything, but what I did do is
still useful because "a) it has to start somewhere, b) it demonstrates
the basic idea of how to structure things."

As far as I'm concerned, I believe the patch series stands on its own
merit. I established the framework to use, and that on its own is useful
and deserves merging, because it means that people can start using it,
and getting things correct from the beginning when adding new code.

Old code does need to be carefully checked, it's not a simple
find/replace, but that can be done incrementally, and I'm willing to
continue work on that myself. I just don't think it has to be all or
nothing at the time of merging.


...

Also, for the record -- while waiting for the Github patch series to be
merged, I did continue to convert more code via git commit --fixup= &&
git rebase -i --autosquash. If it had taken longer to end up being
merged, I would have ended up converting more tests over, and that would
have reflected on the current state of git master.

I'm not sad that it got merged when it was, because again, this work can
be done incrementally and people can take advantage of existing work
immediately. Jens decided it was ready to merge, and that seems like a
fine decision to me. If he had asked me to finish porting all the tests
first, I could have done that too.

More will get done in short order. I'm not even a bottleneck for doing
it. :)

(Though I will work on it.)


-- 
Eli Schwartz

Re: [PATCH for-next 3/3] test range file alloc

[Jens Axboe]

On 6/30/22 1:26 PM, Eli Schwartz wrote:
> On 6/30/22 11:18 AM, Pavel Begunkov wrote:
>> On 6/30/22 15:31, Ammar Faizi wrote:
>>> On 6/30/22 9:19 PM, Pavel Begunkov wrote:
>>>> Nobody cared enough to "fix" all tests to use those new codes, most
>>>> of the cases just return what they've got, but whatever. Same with
>>>> stdout vs stderr.
>>>
>>> That error code rule was invented since commit:
>>>
>>>     68103b731c34a9f83c181cb33eb424f46f3dcb94 ("Merge branch
>>> 'exitcode-protocol' of ....")
>>>
>>>     Ref: https://github.com/axboe/liburing/pull/621/files
>>>
>>> Thanks to Eli who did it. Eli also fixed all tests. Maybe some are still
>>> missing, but if we find it, better to fix it.
>>
>> Have no idea what you're talking about but I'm having
>> hard time calling 6 returns out of 21 in this file "all".
> 
> 
> Hi, I should probably clarify the state of affairs...
> 
> I submitted a patch series on github 4 days ago which implements those
> new codes. It was merged 2 days ago. This is very new code, so I think
> it's not completely 100% fair to say that no one "cared" enough to use it.
> 
> As far as the actual changes and their completion go... take a look at
> the commit messages in the merged patches, specifically take a look at
> commit ed430fbeb33367324a039d9cee0fd504bb91e11a.
> 
> """
> tests: migrate some tests to use enum-based exit codes
> 
> [...]
> 
> A partial migration of existing pass/fail values in test sources is
> included.
> """
> 
> You can also take a look at Github's equivalent of a cover letter, in
> which I mentioned that I haven't ported everything, but what I did do is
> still useful because "a) it has to start somewhere, b) it demonstrates
> the basic idea of how to structure things."
> 
> As far as I'm concerned, I believe the patch series stands on its own
> merit. I established the framework to use, and that on its own is useful
> and deserves merging, because it means that people can start using it,
> and getting things correct from the beginning when adding new code.
> 
> Old code does need to be carefully checked, it's not a simple
> find/replace, but that can be done incrementally, and I'm willing to
> continue work on that myself. I just don't think it has to be all or
> nothing at the time of merging.
> 
> 
> ...
> 
> Also, for the record -- while waiting for the Github patch series to be
> merged, I did continue to convert more code via git commit --fixup= &&
> git rebase -i --autosquash. If it had taken longer to end up being
> merged, I would have ended up converting more tests over, and that would
> have reflected on the current state of git master.
> 
> I'm not sad that it got merged when it was, because again, this work can
> be done incrementally and people can take advantage of existing work
> immediately. Jens decided it was ready to merge, and that seems like a
> fine decision to me. If he had asked me to finish porting all the tests
> first, I could have done that too.

And that was why I merged it, too. I think it's a step in the right
direction, and as long as you keep converting tests so we end up in a
cohesive state, then that's all good. I just did a liburing release and
it'll be at least few months before the next one, now is a good time to
shake up things like this.

Thanks for your work so far, looking forward to the next batch!

-- 
Jens Axboe

Re: [PATCH for-next 3/3] test range file alloc

[Jens Axboe]

On 6/30/22 8:19 AM, Pavel Begunkov wrote:
> On 6/30/22 14:09, Jens Axboe wrote:
>> On 6/30/22 3:13 AM, Pavel Begunkov wrote:
>>> @@ -949,5 +1114,11 @@ int main(int argc, char *argv[])
>>>           return ret;
>>>       }
>>>   +    ret = test_file_alloc_ranges();
>>> +    if (ret) {
>>> +        printf("test_partial_register_fail failed\n");
>>> +        return ret;
>>> +    }
>>
>> If you're returning this directly, test_file_alloc_ranges() should use
>> the proper T_EXIT_foo return codes.
> 
> Nobody cared enough to "fix" all tests to use those new codes, most
> of the cases just return what they've got, but whatever. Same with
> stdout vs stderr.

We'll get there eventually, it's just a bad idea to add NEW tests that
don't adhere to the new rules.

As for stdout vs stderr, by far most of them do it correct. Again, new
tests certainly should.

-- 
Jens Axboe

Re: [PATCH for-next 0/3] ranged file slot alloc

[Pavel Begunkov]

On 6/30/22 10:13, Pavel Begunkov wrote:
> Add helpers and test ranged file slot allocation feature

s/for-next/liburing/ in the subject


> 
> Pavel Begunkov (3):
>    update io_uring.h with file slot alloc ranges
>    alloc range helpers
>    test range file alloc
> 
>   src/include/liburing.h          |   3 +
>   src/include/liburing/io_uring.h |  10 ++
>   src/register.c                  |  14 +++
>   test/file-register.c            | 171 ++++++++++++++++++++++++++++++++
>   4 files changed, 198 insertions(+)
> 

-- 
Pavel Begunkov