Re: [PATCH V2 1/3] ntb: Add a module option to control affinity of DMA channels

[Gary R Hook <ghook@amd.com>]

On 05/04/2017 03:28 PM, Dave Jiang wrote:
>
>
> On 05/04/2017 01:21 PM, Gary R Hook wrote:
>> The DMA channel(s)/memory used to transfer data to an NTB device
>> may not be required to be on the same node as the device. Add a
>> module parameter that allows any candidate channel (aside from
>> node assocation) and allocated memory to be used.
>>
>> Signed-off-by: Gary R Hook <gary.hook@amd.com>
>> ---
>>  drivers/ntb/test/ntb_perf.c |   16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
>> index 42756a98a728..bfceae0e6477 100644
>> --- a/drivers/ntb/test/ntb_perf.c
>> +++ b/drivers/ntb/test/ntb_perf.c
>> @@ -101,6 +101,10 @@
>>  module_param(use_dma, bool, 0644);
>>  MODULE_PARM_DESC(use_dma, "Using DMA engine to measure performance");
>>
>> +static bool on_node = true; /* default to 1 */
>> +module_param(on_node, bool, 0644);
>> +MODULE_PARM_DESC(on_node, "Run threads only on NTB device node (default: true)");
>> +
>>  struct perf_mw {
>>  	phys_addr_t	phys_addr;
>>  	resource_size_t	phys_size;
>> @@ -345,6 +349,10 @@ static int perf_move_data(struct pthr_ctx *pctx, char __iomem *dst, char *src,
>>
>>  static bool perf_dma_filter_fn(struct dma_chan *chan, void *node)
>>  {
>> +	/* Is the channel required to be on the same node as the device? */
>> +	if (!on_node)
>> +		return true;
>> +
>>  	return dev_to_node(&chan->dev->device) == (int)(unsigned long)node;
>>  }
>>
>> @@ -682,7 +690,8 @@ static ssize_t debugfs_run_write(struct file *filp, const char __user *ubuf,
>>  		pr_info("Fix run_order to %u\n", run_order);
>>  	}
>>
>> -	node = dev_to_node(&perf->ntb->pdev->dev);
>> +	node = on_node ? dev_to_node(&perf->ntb->pdev->dev)
>> +		       : NUMA_NO_NODE;
>>  	atomic_set(&perf->tdone, 0);
>>
>>  	/* launch kernel thread */
>> @@ -781,7 +790,10 @@ static int perf_probe(struct ntb_client *client, struct ntb_dev *ntb)
>>
>>  	node = dev_to_node(&pdev->dev);
>>
>> -	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>> +	if (on_node)
>> +		perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>> +	else
>> +		perf = kzalloc(sizeof(*perf), GFP_KERNEL);
>
> Is this necessary when NUMA_NO_NODE is passed in?

Excellent question. I traced kzalloc_node down to slab_alloc_node(), and 
it does
indeed check (parameter) nodeid against NUMA_NO_NODE. So that would be 
an option
for the node variable above.

However, since the code above, as written, doesn't allow for 
NUMA_NO_NODE, some
additional work will be required to properly handle the option. Look for 
a V3 of
this series.

-- 
This is my day job. Follow me at:
IG/Twitter/Facebook: @grhookphoto
IG/Twitter/Facebook: @grhphotographer