Re: [dpdk-dev] [PATCH v3 4/8] raw/ioat: create device on probe and destroy on release

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 28-Jun-19 2:15 PM, Bruce Richardson wrote:
> On Fri, Jun 28, 2019 at 01:59:26PM +0100, Burakov, Anatoly wrote:
>> On 28-Jun-19 1:46 PM, Bruce Richardson wrote:
>>> On Thu, Jun 27, 2019 at 01:28:04PM +0100, Burakov, Anatoly wrote:
>>>> On 27-Jun-19 11:40 AM, Bruce Richardson wrote:
>>>>> Add the create/destroy driver functions so that we can actually allocate
>>>>> a rawdev and destroy it when done. No rawdev API functions are actually
>>>>> implemented at this point.
>>>>>
>>>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>>>>> ---
>>>>
>>>> <snip>
>>>>
>>>>> +	rawdev->driver_name = dev->device.driver->name;
>>>>> +
>>>>> +	ioat = rawdev->dev_private;
>>>>> +	ioat->rawdev = rawdev;
>>>>> +	ioat->regs = dev->mem_resource[0].addr;
>>>>> +	ioat->ring_size = 0;
>>>>> +	ioat->desc_ring = NULL;
>>>>> +	ioat->status_addr = rte_malloc_virt2iova(ioat) +
>>>>> +			offsetof(struct rte_ioat_rawdev, status);
>>>>
>>>> While reviewing other patch, i remembered that i've seen this here. You
>>>> can't make any guarantees about IOVA addresses in rte_malloc-allocated
>>>> memory. Are you sure you don't require IOVA-contiguous memory here?
>>>>
>>> Presumably we can guarantee that for structures less than 1 page in size,
>>> this will work? I believe the device structure should be within that page
>>> limit.
>>>
>>
>> No, we can't. That would only be true if you were allocating IOVA-contiguous
>> memory. Otherwise there's nothing stopping the allocator to allocate even a
>> few kilobytes across page boundary.
>>
>> You can only ever guarantee that *one cache line* will not cross the page
>> boundary with rte_malloc. With rte_memzone and IOVA_CONTIG flag, you'll be
>> able to guarantee IOVA-contiguousness in all cases (or allocation failure).
>>
> Ok, so I either need to move this field to the start of the structure, i.e.
> have offset zero, or else use contiguous allocation. Will fix in next
> version.
> 
> /Bruce
> 

The latter is probably more explicit in intention, i'd rather the code 
not rely on details of rte_malloc implementation :)

-- 
Thanks,
Anatoly