Re: [Intel-gfx] [PATCH v7 00/21] Move all drivers to a common dma-buf locking convention
On 10/18/22 14:34, Christian König wrote: > Am 18.10.22 um 01:07 schrieb Dmitry Osipenko: >> On 10/17/22 20:22, Dmitry Osipenko wrote: >>> Hello, >>> >>> This series moves all drivers to a dynamic dma-buf locking >>> specification. >>> From now on all dma-buf importers are made responsible for holding >>> dma-buf's reservation lock around all operations performed over dma-bufs >>> in accordance to the locking specification. This allows us to utilize >>> reservation lock more broadly around kernel without fearing of a >>> potential >>> deadlocks. >>> >>> This patchset passes all i915 selftests. It was also tested using >>> VirtIO, >>> Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested >>> cases >>> of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where >>> appropriate), >>> which covers majority of kernel drivers since rest of the drivers share >>> same or similar code paths. >>> >>> Changelog: >>> >>> v7: - Rebased on top of recent drm-misc-next. >>> >>> - Added ack from Jason Gunthorpe to the RDMA patch. >>> >>> - Added iosys_map_clear() to dma_buf_vmap_unlocked(), making it >>> fully >>> consistent with dma_buf_vmap(). >>> >>> v6: - Added r-b from Michael Ruhl to the i915 patch. >>> >>> - Added acks from Sumit Semwal and updated commit message of the >>> "Move dma_buf_vmap() to dynamic locking specification" patch like >>> was suggested by Sumit. >>> >>> - Added "!dmabuf" check to dma_buf_vmap_unlocked() to match the >>> locked >>> variant of the function, for consistency. >>> >>> v5: - Added acks and r-bs that were given to v4. >>> >>> - Changed i915 preparation patch like was suggested by Michael >>> Ruhl. >>> The scope of reservation locking is smaller now. >>> >>> v4: - Added dma_buf_mmap() to the "locking convention" documentation, >>> which was missed by accident in v3. >>> >>> - Added acks from Christian König, Tomasz Figa and Hans Verkuil >>> that >>> they gave to couple v3 patches. >>> >>> - Dropped the "_unlocked" postfix from function names that don't >>> have >>> the locked variant, as was requested by Christian König. >>> >>> - Factored out the per-driver preparations into separate patches >>> to ease reviewing of the changes, which is now doable without the >>> global dma-buf functions renaming. >>> >>> - Factored out the dynamic locking convention enforcements into >>> separate >>> patches which add the final dma_resv_assert_held(dmabuf->resv) >>> to the >>> dma-buf API functions. >>> >>> v3: - Factored out dma_buf_mmap_unlocked() and attachment functions >>> into aseparate patches, like was suggested by Christian König. >>> >>> - Corrected and factored out dma-buf locking documentation into >>> a separate patch, like was suggested by Christian König. >>> >>> - Intel driver dropped the reservation locking fews days ago from >>> its BO-release code path, but we need that locking for the >>> imported >>> GEMs because in the end that code path unmaps the imported GEM. >>> So I added back the locking needed by the imported GEMs, updating >>> the "dma-buf attachment locking specification" patch >>> appropriately. >>> >>> - Tested Nouveau+Intel dma-buf import/export combo. >>> >>> - Tested udmabuf import to i915/Nouveau/AMDGPU. >>> >>> - Fixed few places in Etnaviv, Panfrost and Lima drivers that I >>> missed >>> to switch to locked dma-buf vmapping in the drm/gem: Take >>> reservation >>> lock for vmap/vunmap operations" patch. In a result >>> invalidated the >>> Christian's r-b that he gave to v2. >>> >>> - Added locked dma-buf vmap/vunmap functions that are needed for >>> fixing >>> vmappping of Etnaviv, Panfrost and Lima drivers mentioned above. >>> I actually had this change stashed for the drm-shmem shrinker >>> patchset, >>> but then realized that it's already needed by the dma-buf >>> patches. >>> Also improved my tests to better cover these code paths. >>> >>> v2: - Changed locking specification to avoid problems with a >>> cross-driver >>> ww locking, like was suggested by Christian König. Now the >>> attach/detach >>> callbacks are invoked without the held lock and exporter >>> should take the >>> lock. >>> >>> - Added "locking convention" documentation that explains which >>> dma-buf >>> functions and callbacks are locked/unlocked for importers and >>> exporters, >>> which was requested by Christian König. >>> >>> - Added ack from Tomasz Figa to the V4L patches that he gave to v1. >>> >>> Dmitry Osipenko (21): >>> dma-buf: Add unlocked variant of vmapping functions >>> dma-buf: Add unlocked variant of attachment-mapping functions >>> drm/gem: Take reservation lock for vmap/vunmap operations >>> drm/prime: Prepare to dynamic dma-buf locking specification
Re: [Intel-gfx] [PATCH v7 00/21] Move all drivers to a common dma-buf locking convention
Am 18.10.22 um 01:07 schrieb Dmitry Osipenko: On 10/17/22 20:22, Dmitry Osipenko wrote: Hello, This series moves all drivers to a dynamic dma-buf locking specification. From now on all dma-buf importers are made responsible for holding dma-buf's reservation lock around all operations performed over dma-bufs in accordance to the locking specification. This allows us to utilize reservation lock more broadly around kernel without fearing of a potential deadlocks. This patchset passes all i915 selftests. It was also tested using VirtIO, Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested cases of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where appropriate), which covers majority of kernel drivers since rest of the drivers share same or similar code paths. Changelog: v7: - Rebased on top of recent drm-misc-next. - Added ack from Jason Gunthorpe to the RDMA patch. - Added iosys_map_clear() to dma_buf_vmap_unlocked(), making it fully consistent with dma_buf_vmap(). v6: - Added r-b from Michael Ruhl to the i915 patch. - Added acks from Sumit Semwal and updated commit message of the "Move dma_buf_vmap() to dynamic locking specification" patch like was suggested by Sumit. - Added "!dmabuf" check to dma_buf_vmap_unlocked() to match the locked variant of the function, for consistency. v5: - Added acks and r-bs that were given to v4. - Changed i915 preparation patch like was suggested by Michael Ruhl. The scope of reservation locking is smaller now. v4: - Added dma_buf_mmap() to the "locking convention" documentation, which was missed by accident in v3. - Added acks from Christian König, Tomasz Figa and Hans Verkuil that they gave to couple v3 patches. - Dropped the "_unlocked" postfix from function names that don't have the locked variant, as was requested by Christian König. - Factored out the per-driver preparations into separate patches to ease reviewing of the changes, which is now doable without the global dma-buf functions renaming. - Factored out the dynamic locking convention enforcements into separate patches which add the final dma_resv_assert_held(dmabuf->resv) to the dma-buf API functions. v3: - Factored out dma_buf_mmap_unlocked() and attachment functions into aseparate patches, like was suggested by Christian König. - Corrected and factored out dma-buf locking documentation into a separate patch, like was suggested by Christian König. - Intel driver dropped the reservation locking fews days ago from its BO-release code path, but we need that locking for the imported GEMs because in the end that code path unmaps the imported GEM. So I added back the locking needed by the imported GEMs, updating the "dma-buf attachment locking specification" patch appropriately. - Tested Nouveau+Intel dma-buf import/export combo. - Tested udmabuf import to i915/Nouveau/AMDGPU. - Fixed few places in Etnaviv, Panfrost and Lima drivers that I missed to switch to locked dma-buf vmapping in the drm/gem: Take reservation lock for vmap/vunmap operations" patch. In a result invalidated the Christian's r-b that he gave to v2. - Added locked dma-buf vmap/vunmap functions that are needed for fixing vmappping of Etnaviv, Panfrost and Lima drivers mentioned above. I actually had this change stashed for the drm-shmem shrinker patchset, but then realized that it's already needed by the dma-buf patches. Also improved my tests to better cover these code paths. v2: - Changed locking specification to avoid problems with a cross-driver ww locking, like was suggested by Christian König. Now the attach/detach callbacks are invoked without the held lock and exporter should take the lock. - Added "locking convention" documentation that explains which dma-buf functions and callbacks are locked/unlocked for importers and exporters, which was requested by Christian König. - Added ack from Tomasz Figa to the V4L patches that he gave to v1. Dmitry Osipenko (21): dma-buf: Add unlocked variant of vmapping functions dma-buf: Add unlocked variant of attachment-mapping functions drm/gem: Take reservation lock for vmap/vunmap operations drm/prime: Prepare to dynamic dma-buf locking specification drm/armada: Prepare to dynamic dma-buf locking specification drm/i915: Prepare to dynamic dma-buf locking specification drm/omapdrm: Prepare to dynamic dma-buf locking specification drm/tegra: Prepare to dynamic dma-buf locking specification drm/etnaviv: Prepare to dynamic dma-buf locking specification RDMA/umem: Prepare to dynamic dma-buf locking specification misc: fastrpc: Prepare to dynamic dma-buf locking specification xen/gntdev: Prepare to dynamic dma-buf locking
Re: [Intel-gfx] [PATCH v7 00/21] Move all drivers to a common dma-buf locking convention
On 10/17/22 20:22, Dmitry Osipenko wrote: > Hello, > > This series moves all drivers to a dynamic dma-buf locking specification. > From now on all dma-buf importers are made responsible for holding > dma-buf's reservation lock around all operations performed over dma-bufs > in accordance to the locking specification. This allows us to utilize > reservation lock more broadly around kernel without fearing of a potential > deadlocks. > > This patchset passes all i915 selftests. It was also tested using VirtIO, > Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested cases > of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where appropriate), > which covers majority of kernel drivers since rest of the drivers share > same or similar code paths. > > Changelog: > > v7: - Rebased on top of recent drm-misc-next. > > - Added ack from Jason Gunthorpe to the RDMA patch. > > - Added iosys_map_clear() to dma_buf_vmap_unlocked(), making it fully > consistent with dma_buf_vmap(). > > v6: - Added r-b from Michael Ruhl to the i915 patch. > > - Added acks from Sumit Semwal and updated commit message of the > "Move dma_buf_vmap() to dynamic locking specification" patch like > was suggested by Sumit. > > - Added "!dmabuf" check to dma_buf_vmap_unlocked() to match the locked > variant of the function, for consistency. > > v5: - Added acks and r-bs that were given to v4. > > - Changed i915 preparation patch like was suggested by Michael Ruhl. > The scope of reservation locking is smaller now. > > v4: - Added dma_buf_mmap() to the "locking convention" documentation, > which was missed by accident in v3. > > - Added acks from Christian König, Tomasz Figa and Hans Verkuil that > they gave to couple v3 patches. > > - Dropped the "_unlocked" postfix from function names that don't have > the locked variant, as was requested by Christian König. > > - Factored out the per-driver preparations into separate patches > to ease reviewing of the changes, which is now doable without the > global dma-buf functions renaming. > > - Factored out the dynamic locking convention enforcements into separate > patches which add the final dma_resv_assert_held(dmabuf->resv) to the > dma-buf API functions. > > v3: - Factored out dma_buf_mmap_unlocked() and attachment functions > into aseparate patches, like was suggested by Christian König. > > - Corrected and factored out dma-buf locking documentation into > a separate patch, like was suggested by Christian König. > > - Intel driver dropped the reservation locking fews days ago from > its BO-release code path, but we need that locking for the imported > GEMs because in the end that code path unmaps the imported GEM. > So I added back the locking needed by the imported GEMs, updating > the "dma-buf attachment locking specification" patch appropriately. > > - Tested Nouveau+Intel dma-buf import/export combo. > > - Tested udmabuf import to i915/Nouveau/AMDGPU. > > - Fixed few places in Etnaviv, Panfrost and Lima drivers that I missed > to switch to locked dma-buf vmapping in the drm/gem: Take reservation > lock for vmap/vunmap operations" patch. In a result invalidated the > Christian's r-b that he gave to v2. > > - Added locked dma-buf vmap/vunmap functions that are needed for fixing > vmappping of Etnaviv, Panfrost and Lima drivers mentioned above. > I actually had this change stashed for the drm-shmem shrinker patchset, > but then realized that it's already needed by the dma-buf patches. > Also improved my tests to better cover these code paths. > > v2: - Changed locking specification to avoid problems with a cross-driver > ww locking, like was suggested by Christian König. Now the attach/detach > callbacks are invoked without the held lock and exporter should take the > lock. > > - Added "locking convention" documentation that explains which dma-buf > functions and callbacks are locked/unlocked for importers and exporters, > which was requested by Christian König. > > - Added ack from Tomasz Figa to the V4L patches that he gave to v1. > > Dmitry Osipenko (21): > dma-buf: Add unlocked variant of vmapping functions > dma-buf: Add unlocked variant of attachment-mapping functions > drm/gem: Take reservation lock for vmap/vunmap operations > drm/prime: Prepare to dynamic dma-buf locking specification > drm/armada: Prepare to dynamic dma-buf locking specification > drm/i915: Prepare to dynamic dma-buf locking specification > drm/omapdrm: Prepare to dynamic dma-buf locking specification > drm/tegra: Prepare to dynamic dma-buf locking specification > drm/etnaviv: Prepare to dynamic dma-buf locking specification > RDMA/umem: Prepare to dynamic dma-buf locking specification > misc: fastrpc: Prepare
[Intel-gfx] [PATCH v7 00/21] Move all drivers to a common dma-buf locking convention
Hello, This series moves all drivers to a dynamic dma-buf locking specification. >From now on all dma-buf importers are made responsible for holding dma-buf's reservation lock around all operations performed over dma-bufs in accordance to the locking specification. This allows us to utilize reservation lock more broadly around kernel without fearing of a potential deadlocks. This patchset passes all i915 selftests. It was also tested using VirtIO, Panfrost, Lima, Tegra, udmabuf, AMDGPU and Nouveau drivers. I tested cases of display+GPU, display+V4L and GPU+V4L dma-buf sharing (where appropriate), which covers majority of kernel drivers since rest of the drivers share same or similar code paths. Changelog: v7: - Rebased on top of recent drm-misc-next. - Added ack from Jason Gunthorpe to the RDMA patch. - Added iosys_map_clear() to dma_buf_vmap_unlocked(), making it fully consistent with dma_buf_vmap(). v6: - Added r-b from Michael Ruhl to the i915 patch. - Added acks from Sumit Semwal and updated commit message of the "Move dma_buf_vmap() to dynamic locking specification" patch like was suggested by Sumit. - Added "!dmabuf" check to dma_buf_vmap_unlocked() to match the locked variant of the function, for consistency. v5: - Added acks and r-bs that were given to v4. - Changed i915 preparation patch like was suggested by Michael Ruhl. The scope of reservation locking is smaller now. v4: - Added dma_buf_mmap() to the "locking convention" documentation, which was missed by accident in v3. - Added acks from Christian König, Tomasz Figa and Hans Verkuil that they gave to couple v3 patches. - Dropped the "_unlocked" postfix from function names that don't have the locked variant, as was requested by Christian König. - Factored out the per-driver preparations into separate patches to ease reviewing of the changes, which is now doable without the global dma-buf functions renaming. - Factored out the dynamic locking convention enforcements into separate patches which add the final dma_resv_assert_held(dmabuf->resv) to the dma-buf API functions. v3: - Factored out dma_buf_mmap_unlocked() and attachment functions into aseparate patches, like was suggested by Christian König. - Corrected and factored out dma-buf locking documentation into a separate patch, like was suggested by Christian König. - Intel driver dropped the reservation locking fews days ago from its BO-release code path, but we need that locking for the imported GEMs because in the end that code path unmaps the imported GEM. So I added back the locking needed by the imported GEMs, updating the "dma-buf attachment locking specification" patch appropriately. - Tested Nouveau+Intel dma-buf import/export combo. - Tested udmabuf import to i915/Nouveau/AMDGPU. - Fixed few places in Etnaviv, Panfrost and Lima drivers that I missed to switch to locked dma-buf vmapping in the drm/gem: Take reservation lock for vmap/vunmap operations" patch. In a result invalidated the Christian's r-b that he gave to v2. - Added locked dma-buf vmap/vunmap functions that are needed for fixing vmappping of Etnaviv, Panfrost and Lima drivers mentioned above. I actually had this change stashed for the drm-shmem shrinker patchset, but then realized that it's already needed by the dma-buf patches. Also improved my tests to better cover these code paths. v2: - Changed locking specification to avoid problems with a cross-driver ww locking, like was suggested by Christian König. Now the attach/detach callbacks are invoked without the held lock and exporter should take the lock. - Added "locking convention" documentation that explains which dma-buf functions and callbacks are locked/unlocked for importers and exporters, which was requested by Christian König. - Added ack from Tomasz Figa to the V4L patches that he gave to v1. Dmitry Osipenko (21): dma-buf: Add unlocked variant of vmapping functions dma-buf: Add unlocked variant of attachment-mapping functions drm/gem: Take reservation lock for vmap/vunmap operations drm/prime: Prepare to dynamic dma-buf locking specification drm/armada: Prepare to dynamic dma-buf locking specification drm/i915: Prepare to dynamic dma-buf locking specification drm/omapdrm: Prepare to dynamic dma-buf locking specification drm/tegra: Prepare to dynamic dma-buf locking specification drm/etnaviv: Prepare to dynamic dma-buf locking specification RDMA/umem: Prepare to dynamic dma-buf locking specification misc: fastrpc: Prepare to dynamic dma-buf locking specification xen/gntdev: Prepare to dynamic dma-buf locking specification media: videobuf2: Prepare to dynamic dma-buf locking specification media: tegra-vde: Prepare to dynamic dma-buf locking specification
