>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf >Of Chris Wilson >Sent: Thursday, October 3, 2019 3:08 AM >To: Ruhl, Michael J <michael.j.ruhl@xxxxxxxxx>; dri- >devel@xxxxxxxxxxxxxxxxxxxxx >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Subject: RE: [PATCH 2/2] drm/mm: Pack allocated/scanned boolean into a >bitfield > >Quoting Ruhl, Michael J (2019-09-16 20:45:14) >> >-----Original Message----- >> >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On >Behalf >> >Of Chris Wilson >> >Sent: Sunday, September 15, 2019 2:46 PM >> >@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, >> >struct drm_mm_node *node) >> > >> > node->mm = mm; >> > >> >+ __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags); >> >> Maybe a silly question(s), but you appear to be mixing atomic bit ops >> (clear_ and test_) with non-atomic bit ops __xxx_bit(). >> >> Should that be a concern? Should that be mention in the commit? > >Generally yes, but this is inside an allocation function so the new node >cannot be accessed concurrently yet (and manipulation of the drm_mm >itself requires external serialisation). Got it. Thanks for the clarification. Mike >The concern is with blocks like > >> >diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c >b/drivers/gpu/drm/vc4/vc4_crtc.c >> >index f1f0a7c87771..b00e20f5ce05 100644 >> >--- a/drivers/gpu/drm/vc4/vc4_crtc.c >> >+++ b/drivers/gpu/drm/vc4/vc4_crtc.c >> >@@ -994,7 +994,7 @@ static void vc4_crtc_destroy_state(struct drm_crtc >> >*crtc, >> > struct vc4_dev *vc4 = to_vc4_dev(crtc->dev); >> > struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(state); >> > >> >- if (vc4_state->mm.allocated) { >> >+ if (drm_mm_node_allocated(&vc4_state->mm)) { >> > unsigned long flags; >> > >> > spin_lock_irqsave(&vc4->hvs->mm_lock, flags); > >where we are testing the bit prior to taking the lock to serialise >removal before free. To avoid the cost of serialising here we have to be >sure that any other thread has completely stopped using the drm_mm_node >when it is marked as released. >-Chris >_______________________________________________ >dri-devel mailing list >dri-devel@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
