Re: [PATCH v2] gpudev: pin GPU memory

[Elena Agostini <eagostini@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

> 04/01/2022 03:41, eagostini@nvidia.com:
> > From: Elena Agostini <eagostini@nvidia.com>
> >
> > Enable the possibility to make a GPU memory area accessible from
> > the CPU.
> >
> > GPU memory has to be allocated via rte_gpu_mem_alloc().
> >
> > This patch allows the gpudev library to pin, through the GPU driver,
> > a chunk of GPU memory and to return a memory pointer usable
> > by the CPU to access the GPU memory area.
> >
> > Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> [...]
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice.
> > + *
> > + * Pin a chunk of GPU memory to make it accessible from the CPU
>
> You should define what means "pin" exactly.
> Which properties should we expect?
>

Thanks for reviewing, this is the kind of discussion I wanted to have.
Maybe "pin" is too GDRCopy specific oriented.
Here I want to make a GPU memory buffer visible from the CPU. In case
of NVIDIA, this means the GPU memory address has to be pinned (virtual address
doesn't change) and dma-mapped.

Maybe the name should be more like rte_gpu_mem_to_cpu() that's more
explicative and generic.

> > + * using the memory pointer returned by the function.
>
> Which function should return the pointer?
> rte_gpu_mem_pin is returning an int.

Oversight, will fix it.

>
>
> > + * GPU memory has to be allocated via rte_gpu_mem_alloc().
>
> Why pinning is not done by rte_gpu_mem_alloc()?
> Should it be a flag?

rte_gpu_mem_alloc() allocate virtual memory on the GPU that doesn't have
to be necessarily shared (pinned) to make it visible from CPU.

>
> > + *
> > + * @param dev_id
> > + *   Device ID requiring pinned memory.
> > + * @param size
> > + *   Number of bytes to pin.
> > + *   Requesting 0 will do nothing.
> > + * @param ptr
> > + *   Pointer to the GPU memory area to be pinned.
> > + *   NULL is a no-op accepted value.
> > +
> > + * @return
> > + *   A pointer to the pinned GPU memory usable by the CPU, otherwise NULL and rte_errno is set:
> > + *   - ENODEV if invalid dev_id
> > + *   - EINVAL if reserved flags
>
> Which reserved flags?
>
> > + *   - ENOTSUP if operation not supported by the driver
> > + *   - E2BIG if size is higher than limit
> > + *   - ENOMEM if out of space
>
> Is out of space relevant for pinning?

Yes, let me add it

>
> > + *   - EPERM if driver error
> > + */
> > +__rte_experimental
> > +int rte_gpu_mem_pin(int16_t dev_id, size_t size, void *ptr);

[-- Attachment #2: Type: text/html, Size: 10199 bytes --]