Re: [PATCH for-5.15 v2] io_uring: consider cgroup setting when binding sqpoll cpu

在 2021/8/28 上午1:09, Jens Axboe 写道:

On 8/27/21 11:03 AM, Hao Xu wrote:

quoted

在 2021/8/28 上午12:57, Hao Xu 写道:

quoted

在 2021/8/27 下午10:18, Jens Axboe 写道:

quoted

On 8/27/21 8:13 AM, Hao Xu wrote:

quoted

Since sqthread is userspace like thread now, it should respect cgroup
setting, thus we should consider current allowed cpuset when doing
cpu binding for sqthread.

In general, this looks way better than v1. Just a few minor comments
below.

quoted

@@ -7000,6 +7001,16 @@ static bool io_sqd_handle_event(struct

io_sq_data *sqd)
       return did_sig || test_bit(IO_SQ_THREAD_SHOULD_STOP, &sqd->state);
   }
+static int io_sq_bind_cpu(int cpu)
+{
+    if (!test_cpu_in_current_cpuset(cpu))
+        pr_warn("sqthread %d: bound cpu not allowed\n", current->pid);
+    else
+        set_cpus_allowed_ptr(current, cpumask_of(cpu));
+
+    return 0;
+}

This should not be triggerable, unless the set changes between creation
and the thread being created. Hence maybe the warn is fine. I'd probably
prefer terminating the thread at that point, which would result in an
-EOWNERDEAD return when someone attempts to wake the thread.

Which is probably OK, as we really should not hit this path.

Actually I think cpuset change offen happen in container environment(
at leaset in my practice), eg. by resource monitor and balancer. So I
did this check to make sure we are still maintain sq_cpu logic at that
time as possible as we can. Though the problem is still there during
sqthread running time(the cpuset can change at anytime, which changes
the cpumask of sqthread)

And because the cpumask of sqthread may be changed by the cgroup cpuset
change at any time, so here I just print a warnning rather than
terminating sqthread due to this 'normal thing'..

Do we even want the warning then? If it's an expected thing, seems very
annoying to warn about it.Hmm, there are several things:

1) cpuset change may happen several times a day on some environment, so
it doesn't make sense to exit
2) there won't be big chance that the cpuset change happen between
sqthread creation and waken up. So there probably won't be many
warnning.
3) Though we can warn about cpuset change in case 2), but we cannot warn
at the sqthread running time (when it's in while loop) when cpuset
change. And I don't think we should do cpu check and re-binding from
time to time in sqthread. Good thing is users can get this info in
userspace on their own.
So maybe you're right, we should remove this warnning since it doesn't
raise up just for 2), not for all cases, and users can get to know the
situation by taskset command. Jens, What do you think?