Hi Dave,
Thanks for the comment. Maybe it makes sense to add a note (or comment) that choosing CPU from the same NUMA node as the DMA channel is recommended.
Anyway we can assign a different CPU for every DMA channel.
On Mon, Aug 22, 2022 at 08:40:35AM -0700, Dave Jiang wrote:
>
> I don't think this would work for multi-socket systems given when it's
> on you bind all threads to a single CPU even if you may have channels on
> a different NUMA node. One possible way to do this is perhaps using
> on_cpu for Nth core for the NUMA node the channel is on? Just throwing
> out ideas.
>
>
> >
> > Signed-off-by: Alexander Fomichev <a.fomichev@xxxxxxxxx>
> > ---
> > drivers/dma/dmatest.c | 23 +++++++++++++++++++++--
> > 1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> > index f696246f57fd..c91cbc9e5d1a 100644
> > --- a/drivers/dma/dmatest.c
> > +++ b/drivers/dma/dmatest.c
> > @@ -89,6 +89,10 @@ static bool polled;
> > module_param(polled, bool, S_IRUGO | S_IWUSR);
> > MODULE_PARM_DESC(polled, "Use polling for completion instead of interrupts");
> >
> > +static int on_cpu = -1;
> > +module_param(on_cpu, int, 0644);
> > +MODULE_PARM_DESC(on_cpu, "Bind CPU to run threads on (default: auto scheduled (-1))");
> > +
> > /**
> > * struct dmatest_params - test parameters.
> > * @buf_size: size of the memcpy test buffer
> > @@ -237,6 +241,7 @@ struct dmatest_thread {
> > struct dmatest_chan {
> > struct list_head node;
> > struct dma_chan *chan;
> > + int cpu;
> > struct list_head threads;
> > };
> >
> > @@ -602,6 +607,7 @@ static int dmatest_func(void *data)
> > ret = -ENOMEM;
> >
> > smp_rmb();
> > +
>
>
> Stray blank line
>
> > thread->pending = false;
> > info = thread->info;
> > params = &info->params;
> > @@ -1010,6 +1016,7 @@ static int dmatest_add_channel(struct dmatest_info *info,
> > struct dmatest_chan *dtc;
> > struct dma_device *dma_dev = chan->device;
> > unsigned int thread_count = 0;
> > + char cpu_str[20];
> > int cnt;
> >
> > dtc = kmalloc(sizeof(struct dmatest_chan), GFP_KERNEL);
> > @@ -1018,6 +1025,13 @@ static int dmatest_add_channel(struct dmatest_info *info,
> > return -ENOMEM;
> > }
> >
> > + memset(cpu_str, 0, sizeof(cpu_str));
> > + if (on_cpu >= nr_cpu_ids || on_cpu < -1)
> > + on_cpu = -1;
> > + dtc->cpu = on_cpu;
> > + if (dtc->cpu != -1)
> > + snprintf(cpu_str, sizeof(cpu_str) - 1, " on CPU #%d", dtc->cpu);
> > +
> > dtc->chan = chan;
> > INIT_LIST_HEAD(&dtc->threads);
> >
> > @@ -1050,8 +1064,8 @@ static int dmatest_add_channel(struct dmatest_info *info,
> > thread_count += cnt > 0 ? cnt : 0;
> > }
> >
> > - pr_info("Added %u threads using %s\n",
> > - thread_count, dma_chan_name(chan));
> > + pr_info("Added %u threads using %s%s\n",
> > + thread_count, dma_chan_name(chan), cpu_str);
> >
> > list_add_tail(&dtc->node, &info->channels);
> > info->nr_channels++;
> > @@ -1125,6 +1139,11 @@ static void run_pending_tests(struct dmatest_info *info)
> >
> > thread_count = 0;
> > list_for_each_entry(thread, &dtc->threads, node) {
> > + if (dtc->cpu != -1) {
> > + if (!thread->pending)
> > + continue;
> > + kthread_bind(thread->task, dtc->cpu);
> > + }
> > wake_up_process(thread->task);
> > thread_count++;
> > }
All the best,
Alexander.
