| Message ID | 20170412191202.22740-5-eric@anholt.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 12, 2017 at 12:12:01PM -0700, Eric Anholt wrote: > If we follow the typical pattern of the base class being the first > member, we can use the default dma_fence_free function. > > Signed-off-by: Eric Anholt <eric@anholt.net> On 3&4: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Russell King <linux+etnaviv@armlinux.org.uk> > Cc: Christian Gmeiner <christian.gmeiner@gmail.com> > Cc: etnaviv@lists.freedesktop.org > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index da48819ff2e6..0d26ca56e94b 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -998,8 +998,8 @@ static void hangcheck_disable(struct etnaviv_gpu *gpu) > > /* fence object management */ > struct etnaviv_fence { > - struct etnaviv_gpu *gpu; > struct dma_fence base; > + struct etnaviv_gpu *gpu; > }; > > static inline struct etnaviv_fence *to_etnaviv_fence(struct dma_fence *fence) > @@ -1031,20 +1031,13 @@ static bool etnaviv_fence_signaled(struct dma_fence *fence) > return fence_completed(f->gpu, f->base.seqno); > } > > -static void etnaviv_fence_release(struct dma_fence *fence) > -{ > - struct etnaviv_fence *f = to_etnaviv_fence(fence); > - > - kfree_rcu(f, base.rcu); > -} > - > static const struct dma_fence_ops etnaviv_fence_ops = { > .get_driver_name = etnaviv_fence_get_driver_name, > .get_timeline_name = etnaviv_fence_get_timeline_name, > .enable_signaling = etnaviv_fence_enable_signaling, > .signaled = etnaviv_fence_signaled, > .wait = dma_fence_default_wait, > - .release = etnaviv_fence_release, > + .release = dma_fence_free, > }; > > static struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu) > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Eric, Am Mittwoch, den 12.04.2017, 12:12 -0700 schrieb Eric Anholt: > If we follow the typical pattern of the base class being the first > member, we can use the default dma_fence_free function. Sorry, I don't like this change. While it provides a bit of code simplification, it also bakes the implicit assumption into the code that dma_fence is the first struct member without any checks to validate this assumption. I would at least expect a build bug assert, but then the current bit of code to keep this explicit and self-documenting is probably just as good. Regards, Lucas > Signed-off-by: Eric Anholt <eric@anholt.net> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Russell King <linux+etnaviv@armlinux.org.uk> > Cc: Christian Gmeiner <christian.gmeiner@gmail.com> > Cc: etnaviv@lists.freedesktop.org > --- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index da48819ff2e6..0d26ca56e94b 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -998,8 +998,8 @@ static void hangcheck_disable(struct etnaviv_gpu *gpu) > > /* fence object management */ > struct etnaviv_fence { > - struct etnaviv_gpu *gpu; > struct dma_fence base; > + struct etnaviv_gpu *gpu; > }; > > static inline struct etnaviv_fence *to_etnaviv_fence(struct dma_fence *fence) > @@ -1031,20 +1031,13 @@ static bool etnaviv_fence_signaled(struct dma_fence *fence) > return fence_completed(f->gpu, f->base.seqno); > } > > -static void etnaviv_fence_release(struct dma_fence *fence) > -{ > - struct etnaviv_fence *f = to_etnaviv_fence(fence); > - > - kfree_rcu(f, base.rcu); > -} > - > static const struct dma_fence_ops etnaviv_fence_ops = { > .get_driver_name = etnaviv_fence_get_driver_name, > .get_timeline_name = etnaviv_fence_get_timeline_name, > .enable_signaling = etnaviv_fence_enable_signaling, > .signaled = etnaviv_fence_signaled, > .wait = dma_fence_default_wait, > - .release = etnaviv_fence_release, > + .release = dma_fence_free, > }; > > static struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu)
Lucas Stach <l.stach@pengutronix.de> writes: > Hi Eric, > > Am Mittwoch, den 12.04.2017, 12:12 -0700 schrieb Eric Anholt: >> If we follow the typical pattern of the base class being the first >> member, we can use the default dma_fence_free function. > > Sorry, I don't like this change. > While it provides a bit of code simplification, it also bakes the > implicit assumption into the code that dma_fence is the first struct > member without any checks to validate this assumption. I would at least > expect a build bug assert, but then the current bit of code to keep this > explicit and self-documenting is probably just as good. Fine with me. Please make sure you grab patch 2, though.
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c index da48819ff2e6..0d26ca56e94b 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c @@ -998,8 +998,8 @@ static void hangcheck_disable(struct etnaviv_gpu *gpu) /* fence object management */ struct etnaviv_fence { - struct etnaviv_gpu *gpu; struct dma_fence base; + struct etnaviv_gpu *gpu; }; static inline struct etnaviv_fence *to_etnaviv_fence(struct dma_fence *fence) @@ -1031,20 +1031,13 @@ static bool etnaviv_fence_signaled(struct dma_fence *fence) return fence_completed(f->gpu, f->base.seqno); } -static void etnaviv_fence_release(struct dma_fence *fence) -{ - struct etnaviv_fence *f = to_etnaviv_fence(fence); - - kfree_rcu(f, base.rcu); -} - static const struct dma_fence_ops etnaviv_fence_ops = { .get_driver_name = etnaviv_fence_get_driver_name, .get_timeline_name = etnaviv_fence_get_timeline_name, .enable_signaling = etnaviv_fence_enable_signaling, .signaled = etnaviv_fence_signaled, .wait = dma_fence_default_wait, - .release = etnaviv_fence_release, + .release = dma_fence_free, }; static struct dma_fence *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu)
If we follow the typical pattern of the base class being the first member, we can use the default dma_fence_free function. Signed-off-by: Eric Anholt <eric@anholt.net> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Russell King <linux+etnaviv@armlinux.org.uk> Cc: Christian Gmeiner <christian.gmeiner@gmail.com> Cc: etnaviv@lists.freedesktop.org --- drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-)