On 5/7/21 9:19 AM, Pavel Begunkov wrote: > On 5/6/21 8:20 PM, Hao Xu wrote: >> 在 2021/5/7 上午1:10, Jens Axboe 写道: >>> On 5/6/21 8:33 AM, Hao Xu wrote: >>>> Users may want a higher priority for sq_thread or io-worker. Provide a >>>> way to change the nice value(for SCHED_NORMAL) or scheduling policy. >>> >>> Silly question - why is this needed for sqpoll? With the threads now >>> being essentially user threads, why can't we just modify nice and >>> scheduler class from userspace instead? That should work now. I think >>> this is especially true for sqpoll where it's persistent, and argument >>> could be made for the io-wq worker threads that we'd need io_uring >>> support for that, as they come and go and there's no reliable way to >>> find and tweak the thread scheduler settings for that particular use >>> case. >>> >>> It may be more convenient to support this through io_uring, and that is >>> a valid argument. I do think that the better way would then be to simply >>> pass back the sqpoll pid after ring setup, because then it'd almost be >>> as simple to do it from the app itself using the regular system call >>> interfaces for that. >>>> It's my bad. I didn't realize this until I almost completed the patch, >> then I looked into io_uring_param, found just __u32 resv[3] can be >> leveraged. Not sure if it's neccessary to occupy one to do this, so I >> still sent this patch for comments. > > io_uring_param is not a problem, can be extended. > >>> In summary, I do think this _may_ make sense for the worker threads, >>> being able to pass in this information and have io-wq worker thread >>> setup perform the necessary tweaks when a thread is created, but it does >> I'm working on this(for the io-wq worker), have done part of it. > > I'm not sure the io-wq part makes much sense, > > 1) they are per thread, so an instance not related to some particular > ring, and so should not be controlled by it. E.g. what if a ring > has two different rings and sets different schedulers? > > 2) io-wq is slow path in any case, don't think it's worth trinking > with it. Right, it's normally slow path, so perhaps you'd want to nice it down or use a different scheduler class for it. I'm not personally seeing a strong need, but willing to entertain use cases if valid. >>> seem a bit silly to add this for sqpoll where it could just as easily be >>> achieved from the application itself without needing to add this >> It's beyond my knowledge, correct me if I'm wrong: if we do >> it from application, we have to search the pid of sq_thread by it's name >> which is iou-sqp-`sqd->task_pid`, and may be cut off because of >> TASK_COMM_LEN(would this macro value be possibly changed in the >> future?). And set_task_comm() is called when sq_thread runs, so there is >> very small chance(but there is) that set_task_comm() hasn't been called >> when application try to get the command name of sq_thread. Based on this >> (if it is not wrong...) I think return pid of sq_thread in io_uring >> level may be a better choice. > > Right, we may return some id of sqpoll task back in io_uring_param, > though we need to be careful with namespaces. Yep -- Jens Axboe
