2024-03-12 19:45:28

by Xin Wang

Subject: [PATCH] io_uring: extract the function that checks the legitimacy of sq/cq entries

In the io_uring_create function, the sq_entries and cq_entries passed
in by the user are examined. The checking logic is the same for both, so
the common code can be extracted for reuse.

Extract the common code as io_validate_entries function.

Signed-off-by: Xin Wang <[email protected]>
---
io_uring/io_uring.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index cd9a137ad6ce..c51100f39cbf 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3819,6 +3819,18 @@ static struct file *io_uring_get_file(struct io_ring_ctx *ctx)
O_RDWR | O_CLOEXEC, NULL);
}

+static bool io_validate_entries(unsigned int *entries, unsigned int max_entries, __u32 flags)
+{
+ if (!(*entries))
+ return false;
+ if (*entries > max_entries) {
+ if (!(flags & IORING_SETUP_CLAMP))
+ return false;
+ *entries = max_entries;
+ }
+ return true;
+}
+
static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
struct io_uring_params __user *params)
{
@@ -3827,13 +3839,8 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
struct file *file;
int ret;

- if (!entries)
+ if (!io_validate_entries(&entries, IORING_MAX_ENTRIES, p->flags))
return -EINVAL;
- if (entries > IORING_MAX_ENTRIES) {
- if (!(p->flags & IORING_SETUP_CLAMP))
- return -EINVAL;
- entries = IORING_MAX_ENTRIES;
- }

if ((p->flags & IORING_SETUP_REGISTERED_FD_ONLY)
&& !(p->flags & IORING_SETUP_NO_MMAP))
@@ -3854,13 +3861,8 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
* to a power-of-two, if it isn't already. We do NOT impose
* any cq vs sq ring sizing.
*/
- if (!p->cq_entries)
+ if (!io_validate_entries(&(p->cq_entries), IORING_MAX_CQ_ENTRIES, p->flags))
return -EINVAL;
- if (p->cq_entries > IORING_MAX_CQ_ENTRIES) {
- if (!(p->flags & IORING_SETUP_CLAMP))
- return -EINVAL;
- p->cq_entries = IORING_MAX_CQ_ENTRIES;
- }
p->cq_entries = roundup_pow_of_two(p->cq_entries);
if (p->cq_entries < p->sq_entries)
return -EINVAL;
--
2.25.1

2024-03-14 19:29:29

by Jens Axboe

Subject: Re: [PATCH] io_uring: extract the function that checks the legitimacy of sq/cq entries

On 3/12/24 1:44 PM, Xin Wang wrote:
> In the io_uring_create function, the sq_entries and cq_entries passed
> in by the user are examined. The checking logic is the same for both, so
> the common code can be extracted for reuse.

Looks fine to me, though not sure how helpful it really is, it's not
like it's a lot of code and it's easy enough to read as it is. However,
a few minor comments:

> O_RDWR | O_CLOEXEC, NULL);
> }
>
> +static bool io_validate_entries(unsigned int *entries, unsigned int max_entries, __u32 flags)

Line too long, please break list other functions. Also needs a better
name, probably io_validate_ring_entries() would be better.

> +{
> + if (!(*entries))
> + return false;
> + if (*entries > max_entries) {
> + if (!(flags & IORING_SETUP_CLAMP))
> + return false;
> + *entries = max_entries;
> + }
> + return true;
> +}

And I don't know why you use parens for the first *entries check, but
then not for the next? Should be consistent, at least.

> @@ -3854,13 +3861,8 @@ static __cold int io_uring_create(unsigned entries, struct io_uring_params *p,
> * to a power-of-two, if it isn't already. We do NOT impose
> * any cq vs sq ring sizing.
> */
> - if (!p->cq_entries)
> + if (!io_validate_entries(&(p->cq_entries), IORING_MAX_CQ_ENTRIES, p->flags))

Again not sure what these parens are doing here?

--
Jens Axboe