Re: [PATCH V5 07/10] io_uring: switch to kernel_worker

2021-11-22 Thread michael . christie
On 11/22/21 8:20 AM, Jens Axboe wrote:
> On 11/22/21 3:02 AM, Christian Brauner wrote:
>> On Sun, Nov 21, 2021 at 11:17:11AM -0700, Jens Axboe wrote:
>>> On 11/21/21 10:49 AM, Mike Christie wrote:
 Convert io_uring and io-wq to use kernel_worker.
>>>
>>> I don't like the kernel_worker name, that implies it's always giving you
>>> a kernel thread or kthread. That's not the io_uring use case, it's
>>> really just a thread off the original task that just happens to never
>>> exit to userspace.
>>>
>>> Can we do a better name? At least io_thread doesn't imply that.
>>
>> Yeah, I had thought about that as well and at first had kernel_uworker()
>> locally but wasn't convinced. Maybe we should just make it
>> create_user_worker()?
> 
> That's better, or maybe even create_user_inkernel_thread() or something?
> Pretty long, though... I'd be fine with create_user_worker().
> 

Ok, I'll do:

create_user_worker()
start_user_worker()

since you guys agree. It will also match the PF flag naming.

I'll also add more details to the commit message you requested.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V5 07/10] io_uring: switch to kernel_worker

2021-11-22 Thread Jens Axboe
On 11/22/21 3:02 AM, Christian Brauner wrote:
> On Sun, Nov 21, 2021 at 11:17:11AM -0700, Jens Axboe wrote:
>> On 11/21/21 10:49 AM, Mike Christie wrote:
>>> Convert io_uring and io-wq to use kernel_worker.
>>
>> I don't like the kernel_worker name, that implies it's always giving you
>> a kernel thread or kthread. That's not the io_uring use case, it's
>> really just a thread off the original task that just happens to never
>> exit to userspace.
>>
>> Can we do a better name? At least io_thread doesn't imply that.
> 
> Yeah, I had thought about that as well and at first had kernel_uworker()
> locally but wasn't convinced. Maybe we should just make it
> create_user_worker()?

That's better, or maybe even create_user_inkernel_thread() or something?
Pretty long, though... I'd be fine with create_user_worker().

-- 
Jens Axboe

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH V5 07/10] io_uring: switch to kernel_worker

2021-11-21 Thread Jens Axboe
On 11/21/21 10:49 AM, Mike Christie wrote:
> Convert io_uring and io-wq to use kernel_worker.

I don't like the kernel_worker name, that implies it's always giving you
a kernel thread or kthread. That's not the io_uring use case, it's
really just a thread off the original task that just happens to never
exit to userspace.

Can we do a better name? At least io_thread doesn't imply that.

Also I do think this deserves a bit more commit message, there's really
nothing in here.

-- 
Jens Axboe

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH V5 07/10] io_uring: switch to kernel_worker

2021-11-21 Thread Mike Christie
Convert io_uring and io-wq to use kernel_worker.

Signed-off-by: Mike Christie 
Reviewed-by: Christian Brauner 
Reviewed-by: Jens Axboe 
Reviewed-by: Christoph Hellwig 
---
 fs/io-wq.c| 15 ---
 fs/io_uring.c | 11 +--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/io-wq.c b/fs/io-wq.c
index 88202de519f6..b39cf045d6ce 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -70,6 +70,9 @@ struct io_worker {
 
 #define IO_WQ_NR_HASH_BUCKETS  (1u << IO_WQ_HASH_ORDER)
 
+#define IO_WQ_CLONE_FLAGS  (CLONE_FS | CLONE_FILES | CLONE_SIGHAND | \
+CLONE_THREAD | CLONE_IO)
+
 struct io_wqe_acct {
unsigned nr_workers;
unsigned max_workers;
@@ -600,13 +603,9 @@ static int io_wqe_worker(void *data)
struct io_wqe *wqe = worker->wqe;
struct io_wq *wq = wqe->wq;
bool last_timeout = false;
-   char buf[TASK_COMM_LEN];
 
worker->flags |= (IO_WORKER_F_UP | IO_WORKER_F_RUNNING);
 
-   snprintf(buf, sizeof(buf), "iou-wrk-%d", wq->task->pid);
-   set_task_comm(current, buf);
-
audit_alloc_kernel(current);
 
while (!test_bit(IO_WQ_BIT_EXIT, >state)) {
@@ -704,7 +703,7 @@ static void io_init_new_worker(struct io_wqe *wqe, struct 
io_worker *worker,
list_add_tail_rcu(>all_list, >all_list);
worker->flags |= IO_WORKER_F_FREE;
raw_spin_unlock(>lock);
-   wake_up_new_task(tsk);
+   kernel_worker_start(tsk, "iou-wrk-%d", wqe->wq->task->pid);
 }
 
 static bool io_wq_work_match_all(struct io_wq_work *work, void *data)
@@ -734,7 +733,8 @@ static void create_worker_cont(struct callback_head *cb)
worker = container_of(cb, struct io_worker, create_work);
clear_bit_unlock(0, >create_state);
wqe = worker->wqe;
-   tsk = create_io_thread(io_wqe_worker, worker, wqe->node);
+   tsk = kernel_worker(io_wqe_worker, worker, wqe->node,
+   IO_WQ_CLONE_FLAGS, KERN_WORKER_IO);
if (!IS_ERR(tsk)) {
io_init_new_worker(wqe, worker, tsk);
io_worker_release(worker);
@@ -801,7 +801,8 @@ static bool create_io_worker(struct io_wq *wq, struct 
io_wqe *wqe, int index)
if (index == IO_WQ_ACCT_BOUND)
worker->flags |= IO_WORKER_F_BOUND;
 
-   tsk = create_io_thread(io_wqe_worker, worker, wqe->node);
+   tsk = kernel_worker(io_wqe_worker, worker, wqe->node, IO_WQ_CLONE_FLAGS,
+   KERN_WORKER_IO);
if (!IS_ERR(tsk)) {
io_init_new_worker(wqe, worker, tsk);
} else if (!io_should_retry_thread(PTR_ERR(tsk))) {
diff --git a/fs/io_uring.c b/fs/io_uring.c
index b07196b4511c..d01eef85cc03 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7431,12 +7431,8 @@ static int io_sq_thread(void *data)
struct io_sq_data *sqd = data;
struct io_ring_ctx *ctx;
unsigned long timeout = 0;
-   char buf[TASK_COMM_LEN];
DEFINE_WAIT(wait);
 
-   snprintf(buf, sizeof(buf), "iou-sqp-%d", sqd->task_pid);
-   set_task_comm(current, buf);
-
if (sqd->sq_cpu != -1)
set_cpus_allowed_ptr(current, cpumask_of(sqd->sq_cpu));
else
@@ -8665,6 +8661,8 @@ static __cold int io_sq_offload_create(struct io_ring_ctx 
*ctx,
fdput(f);
}
if (ctx->flags & IORING_SETUP_SQPOLL) {
+   unsigned long flags = CLONE_FS | CLONE_FILES | CLONE_SIGHAND |
+   CLONE_THREAD | CLONE_IO;
struct task_struct *tsk;
struct io_sq_data *sqd;
bool attached;
@@ -8710,7 +8708,8 @@ static __cold int io_sq_offload_create(struct io_ring_ctx 
*ctx,
 
sqd->task_pid = current->pid;
sqd->task_tgid = current->tgid;
-   tsk = create_io_thread(io_sq_thread, sqd, NUMA_NO_NODE);
+   tsk = kernel_worker(io_sq_thread, sqd, NUMA_NO_NODE,
+   flags, KERN_WORKER_IO);
if (IS_ERR(tsk)) {
ret = PTR_ERR(tsk);
goto err_sqpoll;
@@ -8718,7 +8717,7 @@ static __cold int io_sq_offload_create(struct io_ring_ctx 
*ctx,
 
sqd->thread = tsk;
ret = io_uring_alloc_task_context(tsk, ctx);
-   wake_up_new_task(tsk);
+   kernel_worker_start(tsk, "iou-sqp-%d", sqd->task_pid);
if (ret)
goto err;
} else if (p->flags & IORING_SETUP_SQ_AFF) {
-- 
2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization