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

[hao.xiang@linux.dev]

> 
> 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
> >
>