[dpdk-dev] [PATCH v13 4/6] dmadev: introduce DMA device library implementation

[fengchengwen]

On 2021/8/5 20:56, Walsh, Conor wrote:
>> This patch introduce DMA device library implementation which includes
>> configuration and I/O with the DMA devices.

[snip]

>>
>>  /**
>>   * @warning
>> @@ -952,10 +1029,27 @@ rte_dmadev_completed(uint16_t dev_id,
>> uint16_t vchan, const uint16_t nb_cpls,
>>   *   status array are also set.
>>   */
> 
> Hi Chenwen,
> 
> When completed status is called with status set to NULL the drivers will segfault.
> Users may have a valid use case where they pass NULL as status so it needs to be
> checked and handled appropriately.
> Could you handle this within dmadev similar to what I've added below?
> If added the doxygen comment will also need to be updated to specify NULL as a valid input.

Hi Conor,

The status must be an array pointer, so below status_tmp will not work well.

This API is slow path (vs completed API), and is designed to obtain detailed
status information, so application should pass valid status parameters.

> 
> Thanks,
> Conor.
> 
>>  __rte_experimental
>> -uint16_t
>> +static inline uint16_t
>>  rte_dmadev_completed_status(uint16_t dev_id, uint16_t vchan,
>>  			    const uint16_t nb_cpls, uint16_t *last_idx,
>> -			    enum rte_dma_status_code *status);
>> +			    enum rte_dma_status_code *status)
>> +{
>> +	struct rte_dmadev *dev = &rte_dmadevices[dev_id];
>> +	uint16_t idx;
>                enum rte_dma_status_code *status_tmp;
>> +
>> +#ifdef RTE_DMADEV_DEBUG
>> +	if (!rte_dmadev_is_valid_dev(dev_id) ||
>> +	    vchan >= dev->data->dev_conf.max_vchans ||
>> +	    nb_cpls == 0 || status == NULL)
>> +		return 0;
>> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->completed_status, 0);
>> +#endif
>> +
>> +	if (last_idx == NULL)
>> +		last_idx = &idx;
>                if (status == NULL)
>                              status = &status_tmp;
>> +
>> +	return (*dev->completed_status)(dev, vchan, nb_cpls, last_idx,
>> status);
>> +}
>>
>>  #ifdef __cplusplus
>>  }
>> diff --git a/lib/dmadev/rte_dmadev_core.h
>> b/lib/dmadev/rte_dmadev_core.h
>> index 599ab15..9272725 100644
>> --- a/lib/dmadev/rte_dmadev_core.h
>> +++ b/lib/dmadev/rte_dmadev_core.h
>> @@ -177,4 +177,6 @@ struct rte_dmadev {
>>  	uint64_t reserved[2]; /**< Reserved for future fields. */
>>  } __rte_cache_aligned;
>>
>> +extern struct rte_dmadev rte_dmadevices[];
>> +
>>  #endif /* _RTE_DMADEV_CORE_H_ */
>> diff --git a/lib/dmadev/version.map b/lib/dmadev/version.map
>> index 408b93c..86c5e75 100644
>> --- a/lib/dmadev/version.map
>> +++ b/lib/dmadev/version.map
>> @@ -27,6 +27,7 @@ EXPERIMENTAL {
>>  INTERNAL {
>>          global:
>>
>> +	rte_dmadevices;
>>  	rte_dmadev_get_device_by_name;
>>  	rte_dmadev_pmd_allocate;
>>  	rte_dmadev_pmd_release;
>> --
>> 2.8.1
>