| Message ID | 20190221184226.2149-1-willy@infradead.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Convert DRM to XArray | expand |
On Thu, Feb 21, 2019 at 10:41:20AM -0800, Matthew Wilcox wrote: > I intend to remove the IDR and the radix tree interfaces from Linux. > Converting each user from either the IDR or radix tree interface varies > from the trivial 1:1 replacement to a complete rewrite of the locking. > Despite the best efforts of our automated testers (who have caught many > of my mistakes), I cannot claim that my conversions of code are free > from bugs. > > Please check these patches over carefully and test them; there may be > off-by-one errors, locking mistakes, or various other failures on my part. > The patches are based on the latest XArray API which can be found here: > http://git.infradead.org/users/willy/linux-dax.git/shortlog/refs/heads/xarray > which I intend to submit during the upcoming merge window. For testing please cc intel-gfx@lists.freedesktop.org on the entire series, including dependencies, so our CI can pick it up. > > Substantive interface changes > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > - The IDR and radix tree required callers to handle their own locking. > The XArray embeds a spinlock which is taken for modifications to > the data structure; plain lookups occur under the RCU read lock or > under the spinlock. > - You can take the spinlock yourself (xa_lock() and friends) to protect > related data. > - idr_alloc() returned -ENOSPC, radix_tree_insert() returned -EEXIST. > xa_insert() and xa_alloc() return -EBUSY. I think this will upset a few of our tests. Just written some more for drm-lease at least, and those check for the ENOSPC. Not sure yet what to do. > - The search keys which the radix tree calls "tags", the XArray calls > "marks". > - There is no preloading in the XArray API. If your locking is > exceptionally complicated, you may need to use xa_reserve(), but > there are only 6 callers of xa_reserve(), so it's quite uncommon. > - The radix tree provided GFP flags as part of the tree definition; > the XArray (like the IDR) passes GFP flags at the point of allocation. > - radix_tree_insert() of a NULL pointer was not well-specified. The > XArray treats it as reserving the entry (it reads back as NULL but > a subsequent xa_insert() to that slot will fail). > - xa_alloc_cyclic() returns 1 if the allocation wraps, unlike > idr_alloc_cyclic() which provides no indication. > - There is no equivalent to idr_for_each(); the xa_for_each() iterator > is similar to idr_for_each_entry(). > - idr_replace() has no exact equivalent. Some users relied on its exact > semantics of only storing if the entry was non-NULL, but all users of > idr_replace() were able to use xa_store(). Yeah looked good to me. > - The family of radix tree gang lookup functions have been replaced with > xa_extract(). > > Matthew Wilcox (34): > drm: Convert drm_minors_idr to XArray > drm: Convert aux_idr to XArray > drm: Convert object_name_idr to XArray > drm: Convert object_idr to XArray > drm: Convert syncobj_idr to XArray > drm: Convert magic_map to XArray > drm: Convert lessee_idr to XArray > drm: Remove linked lists for lessees > drm: Convert ctx_idr to XArray > drm: Convert tile_idr to XArray > drm: Convert crtc_idr to XArray > drm/agp: Convert bo_list_handles to XArray I did at least read the above, I'll leave all the driver patches for others. At least for now, any leftovers can be smashed into drm-misc. Thanks, Daniel > drm/amdgpu: Convert ctx_handles to XArray > drm/amdgpu: Convert pasid_idr to XArray > drm/amdkfd: Convert event_idr to XArray > drm/amdkfd: Convert alloc_idr to XArray > drm/etnaviv: Convert fence_idr to XArray > drm/i915: Convert handles_vma to XArray > drm/i915: Convert spt_tree to XArray > drm/i915: Convert page_track_tree to XArray > drm/i915: Convert get_page to XArray > drm/i915: Convert object_idr to IDA > drm/i915: Convert context_idr to XArray > drm/i915: Convert metrics_idr to XArray > drm/i915: Convert vgpus_idr to XArray > drm/qxl: Convert release_idr to XArray > drm/qxl: Convert surf_id_idr to XArray > drm/tegra: Convert contexts IDR to XArray > drm/vc4: Convert perfmon IDR to XArray > drm/sis: Convert object_idr to XArray > drm/vgem: Convert fence_idr to XArray > drm/via: Convert object_idr to XArray > drm/vmwgfx: Convert base IDR to XArray > drm/vmwgfx: Convert res_idr to XArray > > Documentation/gpu/todo.rst | 3 - > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 22 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 42 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 23 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_sched.c | 4 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 66 ++---- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +- > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 12 +- > drivers/gpu/drm/amd/amdkfd/kfd_events.c | 71 +++---- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 4 +- > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 32 ++- > drivers/gpu/drm/drm_auth.c | 27 +-- > drivers/gpu/drm/drm_connector.c | 27 +-- > drivers/gpu/drm/drm_context.c | 42 ++-- > drivers/gpu/drm/drm_debugfs.c | 19 +- > drivers/gpu/drm/drm_dp_aux_dev.c | 41 ++-- > drivers/gpu/drm/drm_drv.c | 49 ++--- > drivers/gpu/drm/drm_gem.c | 78 +++---- > drivers/gpu/drm/drm_lease.c | 201 ++++++++---------- > drivers/gpu/drm/drm_mode_config.c | 6 +- > drivers/gpu/drm/drm_mode_object.c | 47 ++-- > drivers/gpu/drm/drm_syncobj.c | 64 ++---- > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 16 +- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 +- > drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 3 +- > drivers/gpu/drm/etnaviv/etnaviv_sched.c | 8 +- > drivers/gpu/drm/i915/gvt/display.c | 5 +- > drivers/gpu/drm/i915/gvt/dmabuf.c | 7 +- > drivers/gpu/drm/i915/gvt/gtt.c | 18 +- > drivers/gpu/drm/i915/gvt/gtt.h | 2 +- > drivers/gpu/drm/i915/gvt/gvt.c | 4 +- > drivers/gpu/drm/i915/gvt/gvt.h | 9 +- > drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- > drivers/gpu/drm/i915/gvt/page_track.c | 6 +- > drivers/gpu/drm/i915/gvt/sched_policy.c | 2 +- > drivers/gpu/drm/i915/gvt/vgpu.c | 24 +-- > drivers/gpu/drm/i915/i915_debugfs.c | 48 +++-- > drivers/gpu/drm/i915/i915_drv.h | 9 +- > drivers/gpu/drm/i915/i915_gem.c | 40 ++-- > drivers/gpu/drm/i915/i915_gem_context.c | 43 ++-- > drivers/gpu/drm/i915/i915_gem_context.h | 6 +- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +- > drivers/gpu/drm/i915/i915_gem_object.h | 6 +- > drivers/gpu/drm/i915/i915_perf.c | 39 ++-- > .../gpu/drm/i915/selftests/i915_gem_context.c | 7 +- > drivers/gpu/drm/i915/selftests/mock_context.c | 2 +- > drivers/gpu/drm/msm/msm_gem_submit.c | 12 +- > drivers/gpu/drm/qxl/qxl_cmd.c | 60 ++---- > drivers/gpu/drm/qxl/qxl_drv.h | 6 +- > drivers/gpu/drm/qxl/qxl_kms.c | 8 +- > drivers/gpu/drm/qxl/qxl_release.c | 54 ++--- > drivers/gpu/drm/sis/sis_drv.c | 4 +- > drivers/gpu/drm/sis/sis_drv.h | 2 +- > drivers/gpu/drm/sis/sis_mm.c | 17 +- > drivers/gpu/drm/tegra/drm.c | 35 ++- > drivers/gpu/drm/v3d/v3d_gem.c | 17 +- > drivers/gpu/drm/vc4/vc4_drv.h | 2 +- > drivers/gpu/drm/vc4/vc4_gem.c | 6 +- > drivers/gpu/drm/vc4/vc4_perfmon.c | 33 ++- > drivers/gpu/drm/vgem/vgem_drv.h | 3 +- > drivers/gpu/drm/vgem/vgem_fence.c | 43 ++-- > drivers/gpu/drm/via/via_drv.h | 2 +- > drivers/gpu/drm/via/via_map.c | 4 +- > drivers/gpu/drm/via/via_mm.c | 11 +- > drivers/gpu/drm/vmwgfx/ttm_object.c | 29 +-- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 9 +- > drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 +- > drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 31 +-- > include/drm/drm_auth.h | 11 +- > include/drm/drm_device.h | 4 +- > include/drm/drm_file.h | 15 +- > include/drm/drm_mode_config.h | 18 +- > 75 files changed, 673 insertions(+), 983 deletions(-) > > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Feb 22, 2019 at 10:54:21AM +0100, Daniel Vetter wrote: > > - idr_alloc() returned -ENOSPC, radix_tree_insert() returned -EEXIST. > > xa_insert() and xa_alloc() return -EBUSY. > > I think this will upset a few of our tests. Just written some more for > drm-lease at least, and those check for the ENOSPC. Not sure yet what to > do. If there's real userspace (not just a test suite) which actually relies on the exact errno returned, we can change the places which currently just return the errno to something like: if (err == -EBUSY) return -ENOSPC; return err; There are actually a number of places in the kernel which do the opposite translation today, presumably because having a program print out "No space left on device" was confusing. If it's only the test-suite which cares, presumably the test suite can be changed to treat EBUSY and ENOSPC as being equivalent errno values for a given test. > I did at least read the above, I'll leave all the driver patches for > others. At least for now, any leftovers can be smashed into drm-misc. Thanks! I'll resend after -rc1.