Re: [External] Re: [PATCH v3 11/20] util/dsa: Implement DSA task asynchronous submission and wait for completion.

[hao . xiang]

> 
> On Fri, Mar 8, 2024 at 2:11 AM Jonathan Cameron
> 
> <Jonathan.Cameron@huawei.com> wrote:
> 
> > 
> > On Thu, 4 Jan 2024 00:44:43 +0000
> > 
> >  Hao Xiang <hao.xiang@bytedance.com> wrote:
> > 
> >  * Add a DSA task completion callback.
> > 
> >  * DSA completion thread will call the tasks's completion callback
> > 
> >  on every task/batch task completion.
> > 
> >  * DSA submission path to wait for completion.
> > 
> >  * Implement CPU fallback if DSA is not able to complete the task.
> > 
> >  Signed-off-by: Hao Xiang <hao.xiang@bytedance.com>
> > 
> >  Signed-off-by: Bryan Zhang <bryan.zhang@bytedance.com>
> > 
> >  Hi,
> > 
> >  One naming comment inline. You had me confused on how you were handling 
> > async
> > 
> >  processing at where this is used. Answer is that I think you aren't!
> > 
> >  +/**
> > 
> >  + * @brief Performs buffer zero comparison on a DSA batch task 
> > asynchronously.
> > 
> >  The hardware may be doing it asynchronously but unless that
> > 
> >  buffer_zero_dsa_wait() call doesn't do what it's name suggests, this 
> > function
> > 
> >  is wrapping the async hardware related stuff to make it synchronous.
> > 
> >  So name it buffer_is_zero_dsa_batch_sync()!
> > 
> >  Jonathan


Thanks for reviewing this. The first completion model I tried was to use a busy 
loop to pull for completion on the submission thread but it turns out to have 
too much unnecessary overhead. Think about 10 threads all submitting tasks and 
we end up having 10 busy loops. I moved the completion work to a dedicated 
thread and named it async! However, the async model doesn't fit well with the 
current live migration thread model so eventually I added a wait on the 
submission thread. It was intended to be async but I agree that it is not 
currently. I will rename it in the next revision.

> > 
> >  + *
> > 
> >  + * @param batch_task A pointer to the batch task.
> > 
> >  + * @param buf An array of memory buffers.
> > 
> >  + * @param count The number of buffers in the array.
> > 
> >  + * @param len The buffer length.
> > 
> >  + *
> > 
> >  + * @return Zero if successful, otherwise non-zero.
> > 
> >  + */
> > 
> >  +int
> > 
> >  +buffer_is_zero_dsa_batch_async(struct dsa_batch_task *batch_task,
> > 
> >  + const void **buf, size_t count, size_t len)
> > 
> >  +{
> > 
> >  + if (count <= 0 || count > batch_task->batch_size) {
> > 
> >  + return -1;
> > 
> >  + }
> > 
> >  +
> > 
> >  + assert(batch_task != NULL);
> > 
> >  + assert(len != 0);
> > 
> >  + assert(buf != NULL);
> > 
> >  +
> > 
> >  + if (count == 1) {
> > 
> >  + /* DSA doesn't take batch operation with only 1 task. */
> > 
> >  + buffer_zero_dsa_async(batch_task, buf[0], len);
> > 
> >  + } else {
> > 
> >  + buffer_zero_dsa_batch_async(batch_task, buf, count, len);
> > 
> >  + }
> > 
> >  +
> > 
> >  + buffer_zero_dsa_wait(batch_task);
> > 
> >  + buffer_zero_cpu_fallback(batch_task);
> > 
> >  +
> > 
> >  + return 0;
> > 
> >  +}
> > 
> >  +
> > 
> >  #endif
> >
>