[PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[Stefan Metzmacher]

---
 fs/io-wq.h               | 10 +++++++++-
 fs/io_uring.c            | 16 ++++++++++++++++
 include/linux/io_uring.h | 25 -------------------------
 3 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/fs/io-wq.h b/fs/io-wq.h
index 1ac2f3248088..80d590564ff9 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -2,7 +2,6 @@
 #define INTERNAL_IO_WQ_H
  #include <linux/refcount.h>
-#include <linux/io_uring.h>
  struct io_wq;
 @@ -21,6 +20,15 @@ enum io_wq_cancel {
 	IO_WQ_CANCEL_NOTFOUND,	/* work not found */
 };
 +struct io_wq_work_node {
+	struct io_wq_work_node *next;
+};
+
+struct io_wq_work_list {
+	struct io_wq_work_node *first;
+	struct io_wq_work_node *last;
+};
+
 static inline void wq_list_add_after(struct io_wq_work_node *node,
 				     struct io_wq_work_node *pos,
 				     struct io_wq_work_list *list)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 642ad08d8964..fd0807a3c9c3 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -454,6 +454,22 @@ struct io_ring_ctx {
 	struct list_head		tctx_list;
 };
 +struct io_uring_task {
+	/* submission side */
+	struct xarray		xa;
+	struct wait_queue_head	wait;
+	void			*last;
+	void			*io_wq;
+	struct percpu_counter	inflight;
+	atomic_t		in_idle;
+	bool			sqpoll;
+
+	spinlock_t		task_lock;
+	struct io_wq_work_list	task_list;
+	unsigned long		task_state;
+	struct callback_head	task_work;
+};
+
 /*
  * First field must be the file pointer in all the
  * iocb unions! See also 'struct kiocb' in <linux/fs.h>
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 9761a0ec9f95..79cde9906be0 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -5,31 +5,6 @@
 #include <linux/sched.h>
 #include <linux/xarray.h>
 -struct io_wq_work_node {
-	struct io_wq_work_node *next;
-};
-
-struct io_wq_work_list {
-	struct io_wq_work_node *first;
-	struct io_wq_work_node *last;
-};
-
-struct io_uring_task {
-	/* submission side */
-	struct xarray		xa;
-	struct wait_queue_head	wait;
-	void			*last;
-	void			*io_wq;
-	struct percpu_counter	inflight;
-	atomic_t		in_idle;
-	bool			sqpoll;
-
-	spinlock_t		task_lock;
-	struct io_wq_work_list	task_list;
-	unsigned long		task_state;
-	struct callback_head	task_work;
-};
-
 #if defined(CONFIG_IO_URING)
 struct sock *io_uring_get_socket(struct file *file);
 void __io_uring_task_cancel(void);
-- 
2.25.1

[PATCH 2/2] io_uring: use typesafe pointers in io_uring_task

[Stefan Metzmacher]

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index fd0807a3c9c3..9b8cd9559584 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -458,8 +458,8 @@ struct io_uring_task {
 	/* submission side */
 	struct xarray		xa;
 	struct wait_queue_head	wait;
-	void			*last;
-	void			*io_wq;
+	const struct io_ring_ctx *last;
+	struct io_wq		*io_wq;
 	struct percpu_counter	inflight;
 	atomic_t		in_idle;
 	bool			sqpoll;
-- 
2.25.1

Re: [PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[Pavel Begunkov]

Both patches are really nice. However, 1/2 doesn't apply
and looks it needs small stylistic changes, see nits below

On 14/03/2021 11:15, Stefan Metzmacher wrote:
> ---
>  fs/io-wq.h               | 10 +++++++++-
>  fs/io_uring.c            | 16 ++++++++++++++++
>  include/linux/io_uring.h | 25 -------------------------
>  3 files changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/io-wq.h b/fs/io-wq.h
> index 1ac2f3248088..80d590564ff9 100644
> --- a/fs/io-wq.h
> +++ b/fs/io-wq.h
> @@ -2,7 +2,6 @@
>  #define INTERNAL_IO_WQ_H
>   #include <linux/refcount.h>
> -#include <linux/io_uring.h>
>   struct io_wq;
>  @@ -21,6 +20,15 @@ enum io_wq_cancel {
>  	IO_WQ_CANCEL_NOTFOUND,	/* work not found */
>  };

newline (nl)?

>  +struct io_wq_work_node {
> +	struct io_wq_work_node *next;
> +};
> +
> +struct io_wq_work_list {
> +	struct io_wq_work_node *first;
> +	struct io_wq_work_node *last;
> +};
> +
>  static inline void wq_list_add_after(struct io_wq_work_node *node,
>  				     struct io_wq_work_node *pos,
>  				     struct io_wq_work_list *list)
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 642ad08d8964..fd0807a3c9c3 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -454,6 +454,22 @@ struct io_ring_ctx {
>  	struct list_head		tctx_list;
>  };

nl

>  +struct io_uring_task {
> +	/* submission side */
> +	struct xarray		xa;
> +	struct wait_queue_head	wait;
> +	void			*last;
> +	void			*io_wq;
> +	struct percpu_counter	inflight;
> +	atomic_t		in_idle;
> +	bool			sqpoll;
> +
> +	spinlock_t		task_lock;
> +	struct io_wq_work_list	task_list;
> +	unsigned long		task_state;
> +	struct callback_head	task_work;
> +};
> +
>  /*
>   * First field must be the file pointer in all the
>   * iocb unions! See also 'struct kiocb' in <linux/fs.h>
> diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
> index 9761a0ec9f95..79cde9906be0 100644
> --- a/include/linux/io_uring.h
> +++ b/include/linux/io_uring.h
> @@ -5,31 +5,6 @@
>  #include <linux/sched.h>
>  #include <linux/xarray.h>
>  -struct io_wq_work_node {
> -	struct io_wq_work_node *next;
> -};
> -
> -struct io_wq_work_list {
> -	struct io_wq_work_node *first;
> -	struct io_wq_work_node *last;
> -};
> -
> -struct io_uring_task {
> -	/* submission side */
> -	struct xarray		xa;
> -	struct wait_queue_head	wait;
> -	void			*last;
> -	void			*io_wq;
> -	struct percpu_counter	inflight;
> -	atomic_t		in_idle;
> -	bool			sqpoll;
> -
> -	spinlock_t		task_lock;
> -	struct io_wq_work_list	task_list;
> -	unsigned long		task_state;
> -	struct callback_head	task_work;
> -};
> -

nl

>  #if defined(CONFIG_IO_URING)
>  struct sock *io_uring_get_socket(struct file *file);
>  void __io_uring_task_cancel(void);
> 

-- 
Pavel Begunkov

Re: [PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[Stefan Metzmacher]

Hi Pavel,

> Both patches are really nice. However, 1/2 doesn't apply
> and looks it needs small stylistic changes, see nits below

It seems the mails got corrupted, I played with git imap-send... :-(

Here a branch with the patches:
https://git.samba.org/?p=metze/linux/wip.git;a=shortlog;h=refs/heads/io_uring-5.12

Should I resend them once I fixed my mail setup?

metze

Re: [PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[Pavel Begunkov]

On 14/03/2021 17:14, Stefan Metzmacher wrote:
> Hi Pavel,
> 
>> Both patches are really nice. However, 1/2 doesn't apply
>> and looks it needs small stylistic changes, see nits below
> 
> It seems the mails got corrupted, I played with git imap-send... :-(
> 
> Here a branch with the patches:
> https://git.samba.org/?p=metze/linux/wip.git;a=shortlog;h=refs/heads/io_uring-5.12
> 
> Should I resend them once I fixed my mail setup?
Who knows. Jens, do you want to take it for 5.12?

I just recently converted tctx->last from file* to ctx* and had to
gaze enough to not screw, in this light 2/2 looks good from the
safety perspective.

-- 
Pavel Begunkov

[PATCH v2 0/2] Some header cleanups

[Stefan Metzmacher]

Hi,

here're a few cleanups in order to make structures as private and
typesafe as possible. It also cleans up the layering from io_uring.c
being on top of io-wq.*.

Changes since v1:

- None, hopefully correct patch format now.

Stefan Metzmacher (2):
  io_uring: remove structures from include/linux/io_uring.h
  io_uring: use typesafe pointers in io_uring_task

 fs/io-wq.h               | 10 +++++++++-
 fs/io_uring.c            | 16 ++++++++++++++++
 include/linux/io_uring.h | 25 -------------------------
 3 files changed, 25 insertions(+), 26 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] io_uring: remove structures from include/linux/io_uring.h

[Stefan Metzmacher]

---
 fs/io-wq.h               | 10 +++++++++-
 fs/io_uring.c            | 16 ++++++++++++++++
 include/linux/io_uring.h | 25 -------------------------
 3 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/fs/io-wq.h b/fs/io-wq.h
index 1ac2f3248088..80d590564ff9 100644
--- a/fs/io-wq.h
+++ b/fs/io-wq.h
@@ -2,7 +2,6 @@
 #define INTERNAL_IO_WQ_H
 
 #include <linux/refcount.h>
-#include <linux/io_uring.h>
 
 struct io_wq;
 
@@ -21,6 +20,15 @@ enum io_wq_cancel {
 	IO_WQ_CANCEL_NOTFOUND,	/* work not found */
 };
 
+struct io_wq_work_node {
+	struct io_wq_work_node *next;
+};
+
+struct io_wq_work_list {
+	struct io_wq_work_node *first;
+	struct io_wq_work_node *last;
+};
+
 static inline void wq_list_add_after(struct io_wq_work_node *node,
 				     struct io_wq_work_node *pos,
 				     struct io_wq_work_list *list)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 58d62dd9f8e4..52b5ed71d770 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -454,6 +454,22 @@ struct io_ring_ctx {
 	struct list_head		tctx_list;
 };
 
+struct io_uring_task {
+	/* submission side */
+	struct xarray		xa;
+	struct wait_queue_head	wait;
+	void			*last;
+	void			*io_wq;
+	struct percpu_counter	inflight;
+	atomic_t		in_idle;
+	bool			sqpoll;
+
+	spinlock_t		task_lock;
+	struct io_wq_work_list	task_list;
+	unsigned long		task_state;
+	struct callback_head	task_work;
+};
+
 /*
  * First field must be the file pointer in all the
  * iocb unions! See also 'struct kiocb' in <linux/fs.h>
diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
index 9761a0ec9f95..79cde9906be0 100644
--- a/include/linux/io_uring.h
+++ b/include/linux/io_uring.h
@@ -5,31 +5,6 @@
 #include <linux/sched.h>
 #include <linux/xarray.h>
 
-struct io_wq_work_node {
-	struct io_wq_work_node *next;
-};
-
-struct io_wq_work_list {
-	struct io_wq_work_node *first;
-	struct io_wq_work_node *last;
-};
-
-struct io_uring_task {
-	/* submission side */
-	struct xarray		xa;
-	struct wait_queue_head	wait;
-	void			*last;
-	void			*io_wq;
-	struct percpu_counter	inflight;
-	atomic_t		in_idle;
-	bool			sqpoll;
-
-	spinlock_t		task_lock;
-	struct io_wq_work_list	task_list;
-	unsigned long		task_state;
-	struct callback_head	task_work;
-};
-
 #if defined(CONFIG_IO_URING)
 struct sock *io_uring_get_socket(struct file *file);
 void __io_uring_task_cancel(void);
-- 
2.25.1

[PATCH v2 2/2] io_uring: use typesafe pointers in io_uring_task

[Stefan Metzmacher]

Signed-off-by: Stefan Metzmacher <metze@samba.org>
---
 fs/io_uring.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 52b5ed71d770..f048c42d8b8f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -458,8 +458,8 @@ struct io_uring_task {
 	/* submission side */
 	struct xarray		xa;
 	struct wait_queue_head	wait;
-	void			*last;
-	void			*io_wq;
+	const struct io_ring_ctx *last;
+	struct io_wq		*io_wq;
 	struct percpu_counter	inflight;
 	atomic_t		in_idle;
 	bool			sqpoll;
-- 
2.25.1

Re: [PATCH 1/2] io_uring: remove structures from include/linux/io_uring.h

[Jens Axboe]

On 3/15/21 5:02 AM, Pavel Begunkov wrote:
> On 14/03/2021 17:14, Stefan Metzmacher wrote:
>> Hi Pavel,
>>
>>> Both patches are really nice. However, 1/2 doesn't apply
>>> and looks it needs small stylistic changes, see nits below
>>
>> It seems the mails got corrupted, I played with git imap-send... :-(
>>
>> Here a branch with the patches:
>> https://git.samba.org/?p=metze/linux/wip.git;a=shortlog;h=refs/heads/io_uring-5.12
>>
>> Should I resend them once I fixed my mail setup?
> Who knows. Jens, do you want to take it for 5.12?
> 
> I just recently converted tctx->last from file* to ctx* and had to
> gaze enough to not screw, in this light 2/2 looks good from the
> safety perspective.

Let's do them for 5.12 - Stefan, can you resend please?

-- 
Jens Axboe

Re: [PATCH v2 0/2] Some header cleanups

[Jens Axboe]

On 3/15/21 5:56 AM, Stefan Metzmacher wrote:
> Hi,
> 
> here're a few cleanups in order to make structures as private and
> typesafe as possible. It also cleans up the layering from io_uring.c
> being on top of io-wq.*.
> 
> Changes since v1:
> 
> - None, hopefully correct patch format now.

And I see you already did resend, thanks I'll take them for 5.12.

-- 
Jens Axboe