| Message ID | 1354805670-302-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 6, 2012 at 3:54 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > As the shrinker may be invoked for the allocation, and it may reap > neighbouring objects in the offset range mm, we need to be careful in > the order in which we allocate the node, search for free space and then > insert the node into the mmap offset range manager. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Dave Airlie <airlied@redhat.com> > Cc: dri-devel@lists.freedesktop.org I think leaking allocation/shrinker details from drm/i915 like that into common (helper) code is a no-go. Which means we need to open-code create_mmap_offset in our code and add a big comment ... -Daniel
On Thu, Dec 6, 2012 at 4:25 PM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Dec 6, 2012 at 3:54 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: >> As the shrinker may be invoked for the allocation, and it may reap >> neighbouring objects in the offset range mm, we need to be careful in >> the order in which we allocate the node, search for free space and then >> insert the node into the mmap offset range manager. >> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Dave Airlie <airlied@redhat.com> >> Cc: dri-devel@lists.freedesktop.org > > I think leaking allocation/shrinker details from drm/i915 like that > into common (helper) code is a no-go. Which means we need to open-code > create_mmap_offset in our code and add a big comment ... Otoh killing the 2-step drm_mm node allocation process with search_free and get_block has been on my todo since forever. Especially since it doesn't add any useful flexibility, but makes preallocation schemes much more likely to be broken. See this patch, or what ttm tries to accomplish (with the broken prealloc helpers in drm_mm.c). So reconsidering this patch it's Reviewed-by: Daniel Vetter <daniel@ffwll.ch> But not exactly for the reasons listed in the commit message ;-) Cheers, Daniel
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 24efae4..3a2d594 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -353,19 +353,17 @@ drm_gem_create_mmap_offset(struct drm_gem_object *obj) map->handle = obj; /* Get a DRM GEM mmap offset allocated... */ - list->file_offset_node = drm_mm_search_free(&mm->offset_manager, - obj->size / PAGE_SIZE, 0, false); - - if (!list->file_offset_node) { - DRM_ERROR("failed to allocate offset for bo %d\n", obj->name); - ret = -ENOSPC; + list->file_offset_node = kzalloc(sizeof(struct drm_mm_node), GFP_KERNEL); + if (list->file_offset_node == NULL) { + ret = -ENOMEM; goto out_free_list; } - list->file_offset_node = drm_mm_get_block(list->file_offset_node, - obj->size / PAGE_SIZE, 0); - if (!list->file_offset_node) { - ret = -ENOMEM; + ret = drm_mm_insert_node(&mm->offset_manager, list->file_offset_node, + obj->size / PAGE_SIZE, 0); + if (ret) { + DRM_ERROR("failed to allocate offset for bo\n"); + kfree(list->file_offset_node); goto out_free_list; }
As the shrinker may be invoked for the allocation, and it may reap neighbouring objects in the offset range mm, we need to be careful in the order in which we allocate the node, search for free space and then insert the node into the mmap offset range manager. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Dave Airlie <airlied@redhat.com> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/drm_gem.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)