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

[Jens Axboe <axboe@kernel.dk>]

On 8/27/21 11:03 AM, Hao Xu wrote:
> 在 2021/8/28 上午12:57, Hao Xu 写道:
>> 在 2021/8/27 下午10:18, Jens Axboe 写道:
>>> On 8/27/21 8:13 AM, Hao Xu wrote:
>>>> 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.
>>>
>>>> @@ -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.

-- 
Jens Axboe