Re: [Intel-gfx] [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops

On 03/10/2019 22:00, Chris Wilson wrote:

A straightforward conversion of assignment and checking of the boolean state flags (allocated, scanned) into non-atomic bitops. The caller remains responsible for all locking around the drm_mm and its nodes.

Signed-off-by: Chris Wilson chris@chris-wilson.co.uk

drivers/gpu/drm/drm_mm.c | 18 +++++++++--------- drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem.c | 4 ++-- include/drm/drm_mm.h | 7 ++++--- 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 99312bdc6273..a9cab5e53731 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -426,7 +426,7 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node)

list_add(&node->node_list, &hole->node_list); drm_mm_interval_tree_add_node(hole, node);

• node->allocated = true;

• __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags); node->hole_size = 0;

  rm_hole(hole);

@@ -545,7 +545,7 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,

list_add(&node->node_list, &hole->node_list);
drm_mm_interval_tree_add_node(hole, node);

• node->allocated = true;
  

• __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  
  

  rm_hole(hole); if (adj_start > hole_start)

@@ -563,7 +563,7 @@ EXPORT_SYMBOL(drm_mm_insert_node_in_range);

static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node) {

• return node->scanned_block;

• return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags); }

  /**

@@ -589,7 +589,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)

drm_mm_interval_tree_remove(node, &mm->interval_tree); list_del(&node->node_list);

• node->allocated = false;

• __clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);

  if (drm_mm_hole_follows(prev_node)) rm_hole(prev_node);

@@ -627,8 +627,8 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new) &mm->holes_addr); }

• old->allocated = false;
• new->allocated = true;

• __clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
• __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags); } EXPORT_SYMBOL(drm_mm_replace_node);

@@ -738,7 +738,7 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan, DRM_MM_BUG_ON(node->mm != mm); DRM_MM_BUG_ON(!drm_mm_node_allocated(node)); DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));

• node->scanned_block = true;

• __set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags); mm->scan_active++;

  /* Remove this block from the node_list so that we enlarge the hole

@@ -824,7 +824,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,

DRM_MM_BUG_ON(node->mm != scan->mm); DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));

• node->scanned_block = false;

• __clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);

  DRM_MM_BUG_ON(!node->mm->scan_active); node->mm->scan_active--;

@@ -922,7 +922,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)

/* Clever trick to avoid a special case in the free hole tracking. */ INIT_LIST_HEAD(&mm->head_node.node_list);

• mm->head_node.allocated = false;

• mm->head_node.flags = 0; mm->head_node.mm = mm; mm->head_node.start = start + size; mm->head_node.size = -size;

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c index 20d8a6297985..c049199a1df5 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c @@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache, cache->use_64bit_reloc = HAS_64BIT_RELOC(i915); cache->has_fence = cache->gen < 4; cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;

• cache->node.allocated = false;

• cache->node.flags = 0; cache->ce = NULL; cache->rq = NULL; cache->rq_size = 0;

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index fa8e028ac0b5..7046067f70c1 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -351,7 +351,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj, PIN_NOEVICT); if (!IS_ERR(vma)) { node.start = i915_ggtt_offset(vma);

• node.allocated = false;
  

• node.flags = 0;
  

  } else { ret = insert_mappable_node(ggtt, &node, PAGE_SIZE); if (ret)

@@ -561,7 +561,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj, PIN_NOEVICT); if (!IS_ERR(vma)) { node.start = i915_ggtt_offset(vma);

• node.allocated = false;
  

• node.flags = 0;
  

  } else { ret = insert_mappable_node(ggtt, &node, PAGE_SIZE); if (ret)

diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index 2c3bbb43c7d1..d7939c054259 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -168,8 +168,9 @@ struct drm_mm_node { struct rb_node rb_hole_addr; u64 __subtree_last; u64 hole_size;

• bool allocated : 1;
• bool scanned_block : 1;

• unsigned long flags;

+#define DRM_MM_NODE_ALLOCATED_BIT 0 +#define DRM_MM_NODE_SCANNED_BIT 1 #ifdef CONFIG_DRM_DEBUG_MM depot_stack_handle_t stack; #endif @@ -253,7 +254,7 @@ struct drm_mm_scan { */ static inline bool drm_mm_node_allocated(const struct drm_mm_node *node) {

• return node->allocated;

• return test_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags); }

  /**

Reviewed-by: Tvrtko Ursulin tvrtko.ursulin@intel.com

Regards,

Tvrtko