| Message ID | 1587056973-101760-1-git-send-email-jianxin.xiong@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RDMA: Add dma-buf support | expand |
On Thu, Apr 16, 2020 at 10:09:30AM -0700, Jianxin Xiong wrote: > This patch set adds dma-buf importer role to the RDMA driver and thus > provides a non-proprietary approach for supporting RDMA to/from buffers > allocated from device local memory (e.g. GPU VRAM). How exactly does this allow access to GPU VRAM? dma_buf_attach() cannot return non-struct page memory in the sgt, and I'm not sure the API has enough information to do use the p2pdma stuff to even establish a p2p mapping. We've already been over this in another thread.. There is a way to improve things to get there, but I don't understand how this patch series is claming to be able to work with VRAM - if it is that means there is a bug in a GPU driver that should be squashed. Other than that, this seems broadly reasonable to me as a way to access a DMA buf pointing at system memory, though it would be nice to have a rational for why we should do this rather than rely on mmap'd versions of a dma buf. Jason
> > This patch set adds dma-buf importer role to the RDMA driver and thus > > provides a non-proprietary approach for supporting RDMA to/from > > buffers allocated from device local memory (e.g. GPU VRAM). > > How exactly does this allow access to GPU VRAM? > > dma_buf_attach() cannot return non-struct page memory in the sgt, and I'm not sure the API has enough information to do use the p2pdma > stuff to even establish a p2p mapping. > > We've already been over this in another thread.. There is a way to improve things to get there, but I don't understand how this patch series > is claming to be able to work with VRAM - if it is that means there is a bug in a GPU driver that should be squashed. > Right, the GPU driver needs to cooperate to get the thing to work as expected. The "p2p" flag and related GPU driver changes proposed in other threads would ensure VRAM is really used. Alternatively, a GPU driver can have a working mode that assumes p2p mapping capability of the client. Either way, the patches to the RDMA driver would be mostly identical except for adding the use of the "p2p" flag. > Other than that, this seems broadly reasonable to me as a way to access a DMA buf pointing at system memory, though it would be nice to > have a rational for why we should do this rather than rely on mmap'd versions of a dma buf. > > Jason
On Thu, Apr 16, 2020 at 07:08:15PM +0000, Xiong, Jianxin wrote: > > > This patch set adds dma-buf importer role to the RDMA driver and thus > > > provides a non-proprietary approach for supporting RDMA to/from > > > buffers allocated from device local memory (e.g. GPU VRAM). > > > > How exactly does this allow access to GPU VRAM? > > > > dma_buf_attach() cannot return non-struct page memory in the sgt, > > and I'm not sure the API has enough information to do use the > > p2pdma stuff to even establish a p2p mapping. > > > > We've already been over this in another thread.. There is a way to > > improve things to get there, but I don't understand how this patch > > series is claming to be able to work with VRAM - if it is that > > means there is a bug in a GPU driver that should be squashed. > > > > Right, the GPU driver needs to cooperate to get the thing to work as > expected. The "p2p" flag and related GPU driver changes proposed in > other threads would ensure VRAM is really used. Alternatively, a > GPU driver can have a working mode that assumes p2p mapping > capability of the client. Either way, the patches to the RDMA driver > would be mostly identical except for adding the use of the "p2p" > flag. I think the other thread has explained this would not be "mostly identical" but here is significant work to rip out the scatter list from the umem. So, I'm back to my original ask, can you justify adding this if it cannot do VRAM? What is the use case? Jason
> > > > Right, the GPU driver needs to cooperate to get the thing to work as > > expected. The "p2p" flag and related GPU driver changes proposed in > > other threads would ensure VRAM is really used. Alternatively, a GPU > > driver can have a working mode that assumes p2p mapping capability of > > the client. Either way, the patches to the RDMA driver would be mostly > > identical except for adding the use of the "p2p" > > flag. > > I think the other thread has explained this would not be "mostly identical" but here is significant work to rip out the scatter list from the > umem. > Probably we are referring to different threads here. Could you kindly refer me to the thread you mentioned? I was referring to the thread about patching dma-buf and GPU driver: https://www.spinics.net/lists/amd-gfx/msg47022.html > So, I'm back to my original ask, can you justify adding this if it cannot do VRAM? What is the use case? Working with VRAM is the goal. This patch has been tested with a modified GPU driver that has dma_buf_ops set up to not migrate the buffer to system memory when attached. The GPU drivers and the RDMA drivers can be improved independently and it doesn't hurt to have the RDMA driver ready before the GPU drivers. > > Jason
On Thu, Apr 16, 2020 at 09:02:51PM +0000, Xiong, Jianxin wrote: > > > > > > Right, the GPU driver needs to cooperate to get the thing to work as > > > expected. The "p2p" flag and related GPU driver changes proposed in > > > other threads would ensure VRAM is really used. Alternatively, a GPU > > > driver can have a working mode that assumes p2p mapping capability of > > > the client. Either way, the patches to the RDMA driver would be mostly > > > identical except for adding the use of the "p2p" > > > flag. > > > > I think the other thread has explained this would not be "mostly identical" but here is significant work to rip out the scatter list from the > > umem. > > > > Probably we are referring to different threads here. Could you > kindly refer me to the thread you mentioned? I was referring to the > thread about patching dma-buf and GPU driver: > https://www.spinics.net/lists/amd-gfx/msg47022.html https://lore.kernel.org/linux-media/20200311152838.GA24280@infradead.org/ > > So, I'm back to my original ask, can you justify adding this if it > > cannot do VRAM? What is the use case? > > Working with VRAM is the goal. This patch has been tested with a > modified GPU driver that has dma_buf_ops set up to not migrate the > buffer to system memory when attached. The GPU drivers and the RDMA > drivers can be improved independently and it doesn't hurt to have > the RDMA driver ready before the GPU drivers. Well, if there is no other use case then this series will have to wait until someone does all the other work to make P2P work upstream. Jason
> > > > > > > > Right, the GPU driver needs to cooperate to get the thing to work > > > > as expected. The "p2p" flag and related GPU driver changes > > > > proposed in other threads would ensure VRAM is really used. > > > > Alternatively, a GPU driver can have a working mode that assumes > > > > p2p mapping capability of the client. Either way, the patches to > > > > the RDMA driver would be mostly identical except for adding the use of the "p2p" > > > > flag. > > > > > > I think the other thread has explained this would not be "mostly > > > identical" but here is significant work to rip out the scatter list from the umem. > > > > > > > Probably we are referring to different threads here. Could you kindly > > refer me to the thread you mentioned? I was referring to the thread > > about patching dma-buf and GPU driver: > > https://www.spinics.net/lists/amd-gfx/msg47022.html > > https://lore.kernel.org/linux-media/20200311152838.GA24280@infradead.org/ > Thanks. We are actually looking at the same series but somehow I skipped the details of the single patch that looks simplest which turns out to have most complication. I agree if scatter list is not to be used, there are going to be significant work involved. > > > So, I'm back to my original ask, can you justify adding this if it > > > cannot do VRAM? What is the use case? > > > > Working with VRAM is the goal. This patch has been tested with a > > modified GPU driver that has dma_buf_ops set up to not migrate the > > buffer to system memory when attached. The GPU drivers and the RDMA > > drivers can be improved independently and it doesn't hurt to have the > > RDMA driver ready before the GPU drivers. > > Well, if there is no other use case then this series will have to wait until someone does all the other work to make P2P work upstream. > > Jason