On 2019/7/30 3:29, Tejun Heo wrote:
> Hello,
> 
> On Wed, Jul 24, 2019 at 05:59:39PM +0000, Biaoxiang Ye wrote:
>> @@ -1441,7 +1444,27 @@ static void __queue_work(int cpu, struct 
>> workqueue_struct *wq,
>>              if (worker && worker->current_pwq->wq == wq) {
>>                      pwq = worker->current_pwq;
>>              } else {
>> -                    /* meh... not running there, queue here */
>> +                    /*
>> +                     * meh... not running there, queue here
>> +                     * we can't break the ordering guarantee of dynamic 
>> single thread wq,
>> +                     * so have to check whethere the work are still pending 
>> in last pool or not.
>> +                     */
>> +                    if (wq->flags & __WQ_DYNAMIC) {
>> +                            list_for_each_entry(work_tmp, 
>> &last_pool->worklist, entry) {
>> +                                    if (work_tmp == work) {
>> +                                            pending = true;
>> +                                            break;
>> +                                    }
>> +                            }
>> +                            if (pending) {
>> +                                    last_pwq = get_work_pwq(work);
>> +                                    if (likely(last_pwq))
>> +                                            pwq = last_pwq;
>> +                                    else    /* queue here */
>> +                                            pr_warn("workqueue: work 
>> pending in last pool, "
>> +                                                            "but can't get 
>> pwq.\n");
>> +                            }
>> +                    }
> 
> So, I'm not against the idea of making ordered workqueues numa-aware
> but this implementation is a bit too ugly.  Maybe the cleanest way to
> implement this is by synchronizing and ordering the pwqs?
> 
> Thanks.
> 
Hello, tejun

Thanks for your comment. Sorry I made a mistake, the above code check pending
is unnecessary, because function queue_work_on already checked it by
WORK_STRUCT_PENDING_BIT, so the above code will never executed.

For single work, the order can guaranteed already, single work will only
queued on single pwq at a time. The challenge is how to guarantee the order
between different works, they may queued on different pwqs, and executed
by each worker without synchronizing.

One of immature scheme is add two members enqueue_num and process_num to WQ,
when a task enqueue in function __queue_work, save the enqueue_num to task,
and then increase enqueue_num. When process_one_work check the enqueue_num of
work whether equal with current_num or not. Keep waitting if false, if true
then execute the work and increase current_num after done.
like below:
__queue_work()
        //store enqueue_num to each work
        if (wq->flags & __WQ_DYNAMIC)
                work->enqueue_num = atomic_inc_return(&wq->enqueue_num);

        insert_work(pwq, work, worklist, work_flags);
        
process_one_work()
        //wait for process
        if (pwq->wq->flags & __WQ_DYNAMIC) {
                while (work->enqueue_num != atomic_read(&pwq->wq->current_num))
                        ndelay(10);
        }       
        worker->current_func(work);
        
        //done one work, increase current_num
        if (pwq->wq->flags & __WQ_DYNAMIC)
             atomic_inc(&pwq->wq->current_num);

This immature scheme seems still too ugly, I tested it and got a smack in the 
eye.
So is there any good idea to making ordered workqueues numa-aware faultlessly?
BTW, I have no idea if is necessary to guarantee the order between different 
works.

Thanks.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/5D3FFCFA.2030001%40huawei.com.

Reply via email to