On 02/04/2021 15:38, Xiaoguang Wang wrote:
> hi,
>
>> On 31/03/2021 16:59, Xiaoguang Wang wrote:
>>> We have already supported multiple rings to share one same poll thread
>>> by passing IORING_SETUP_ATTACH_WQ, but it's not that convenient to use.
>>> IORING_SETUP_ATTACH_WQ needs users to ensure that a parent ring instance
>>> has beed created firstly, that means it will require app to regulate the
>>> creation oder between uring instances.
>>>
>>> Currently we can make this a bit simpler, for those rings which will
>>> have SQPOLL enabled and are willing to be bound to one same cpu, add a
>>> capability that these rings can share one poll thread by specifying
>>> a new IORING_SETUP_SQPOLL_PERCPU flag, then we have 3 cases
>>> 1, IORING_SETUP_ATTACH_WQ: if user specifies this flag, we'll always
>>> try to attach this ring to an existing ring's corresponding poll thread,
>>> no matter whether IORING_SETUP_SQ_AFF or IORING_SETUP_SQPOLL_PERCPU is
>>> set.
>>> 2, IORING_SETUP_SQ_AFF and IORING_SETUP_SQPOLL_PERCPU are both enabled,
>>> for this case, we'll create a single poll thread to be shared by rings
>>> rings which have same sq_thread_cpu.
>>> 3, for any other cases, we'll just create one new poll thread for the
>>> corresponding ring.
>>>
>>> And for case 2, don't need to regulate creation oder of multiple uring
>>> instances, we use a mutex to synchronize creation, for example, say five
>>> rings which all have IORING_SETUP_SQ_AFF & IORING_SETUP_SQPOLL_PERCPU
>>> enabled, and are willing to be bound same cpu, one ring that gets the
>>> mutex lock will create one poll thread, the other four rings will just
>>> attach themselves to the previous created poll thread once they get lock
>>> successfully.
>>>
>>> To implement above function, define below data structs:
>>> struct percpu_sqd_entry {
>>> struct list_head node;
>>> struct io_sq_data *sqd;
>>> pid_t tgid;
>>> };
>>>
>>> struct percpu_sqd_list {
>>> struct list_head head;
>>> struct mutex lock;
>>> };
>>>
>>> static struct percpu_sqd_list __percpu *percpu_sqd_list;
>>>
>>> sqthreads that have same sq_thread_cpu will be linked together in a percpu
>>> percpu_sqd_list's head. When IORING_SETUP_SQ_AFF and IORING_SETUP_SQPOLL_PERCPU
>>> are both enabled, we will use struct io_uring_params' sq_thread_cpu and
>>> current-tgid locate corresponding sqd.
>>
>> I can't help myself but wonder why not something in the userspace like
>> a pseudo-coded snippet below?
> Yes, agree with you, this feature can be done in userspace. Indeed I also don't
> have a much strong preference that this patch is merged into mainline codes, but it's
> really convenient for usrs who want to make multiple rings share one same sqthread
> by specifying cpu id.
>
>>
>> BTW, don't think "pid_t tgid" will work with namespaces/cgroups.
> In copy_process():
> /* ok, now we should be set up.. */
> p->pid = pid_nr(pid);
> if (clone_flags & CLONE_THREAD) {
> p->group_leader = current->group_leader;
> p->tgid = current->tgid;
> } else {
> p->group_leader = p;
> p->tgid = p->pid;
> }
>
> current->tgid comes form pid_nr(pid), pid_nr() returns a global id seen from
> the init namespace, seems that this id is unique. I'll try to confirm this
> assumption more, thanks.
Ah, could be, we definitely need to take a closer look :)
--
Pavel Begunkov
