Am Dienstag, dem 20.06.2023 um 17:47 +0800 schrieb Sui Jingfeng: > From: Sui Jingfeng <suijingfeng@xxxxxxxxxxx> > > There are numerous members in the struct etnaviv_drm_private, which are > shared by all GPU core. This patch introduces two dedicated functions for > the construction and destruction of the instances of this structure. > The goal is to keep its members from leaking to the outside. The code > needed for error handling can also be simplified. > > Cc: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > Cc: Christian Gmeiner <christian.gmeiner@xxxxxxxxx> > Cc: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx> > --- > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 73 +++++++++++++++++---------- > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + > 2 files changed, 47 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index cec005035d0e..6a048be02857 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -24,9 +24,47 @@ > #include "etnaviv_perfmon.h" > > /* > - * DRM operations: > + * etnaviv private data construction and destructions: > */ > +static struct etnaviv_drm_private * > +etnaviv_alloc_private(struct device *dev, struct drm_device *drm) > +{ > + struct etnaviv_drm_private *priv; > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return ERR_PTR(-ENOMEM); > + > + priv->drm = drm; That's an unrelated change that you rely on in later patches. If this is needed at all it needs to be in a separate patch with a explanation on why it is needed. Regards, Lucas > + > + xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC); > + > + mutex_init(&priv->gem_lock); > + INIT_LIST_HEAD(&priv->gem_list); > + priv->num_gpus = 0; > + priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > > + priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(dev); > + if (IS_ERR(priv->cmdbuf_suballoc)) { > + kfree(priv); > + dev_err(dev, "Failed to create cmdbuf suballocator\n"); > + return ERR_PTR(-ENOMEM); > + } > + > + return priv; > +} > + > +static void etnaviv_free_private(struct etnaviv_drm_private *priv) > +{ > + if (!priv) > + return; > + > + etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc); > + > + xa_destroy(&priv->active_contexts); > + > + kfree(priv); > +} > > static void load_gpu(struct drm_device *dev) > { > @@ -511,35 +549,21 @@ static int etnaviv_bind(struct device *dev) > if (IS_ERR(drm)) > return PTR_ERR(drm); > > - priv = kzalloc(sizeof(*priv), GFP_KERNEL); > - if (!priv) { > - dev_err(dev, "failed to allocate private data\n"); > - ret = -ENOMEM; > + priv = etnaviv_alloc_private(dev, drm); > + if (IS_ERR(priv)) { > + ret = PTR_ERR(priv); > goto out_put; > } > + > drm->dev_private = priv; > > dma_set_max_seg_size(dev, SZ_2G); > > - xa_init_flags(&priv->active_contexts, XA_FLAGS_ALLOC); > - > - mutex_init(&priv->gem_lock); > - INIT_LIST_HEAD(&priv->gem_list); > - priv->num_gpus = 0; > - priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN; > - > - priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev); > - if (IS_ERR(priv->cmdbuf_suballoc)) { > - dev_err(drm->dev, "Failed to create cmdbuf suballocator\n"); > - ret = PTR_ERR(priv->cmdbuf_suballoc); > - goto out_free_priv; > - } > - > dev_set_drvdata(dev, drm); > > ret = component_bind_all(dev, drm); > if (ret < 0) > - goto out_destroy_suballoc; > + goto out_free_priv; > > load_gpu(drm); > > @@ -551,10 +575,8 @@ static int etnaviv_bind(struct device *dev) > > out_unbind: > component_unbind_all(dev, drm); > -out_destroy_suballoc: > - etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc); > out_free_priv: > - kfree(priv); > + etnaviv_free_private(priv); > out_put: > drm_dev_put(drm); > > @@ -570,12 +592,9 @@ static void etnaviv_unbind(struct device *dev) > > component_unbind_all(dev, drm); > > - etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc); > - > - xa_destroy(&priv->active_contexts); > + etnaviv_free_private(priv); > > drm->dev_private = NULL; > - kfree(priv); > > drm_dev_put(drm); > } > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > index b3eb1662e90c..e58f82e698de 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > @@ -35,6 +35,7 @@ struct etnaviv_file_private { > }; > > struct etnaviv_drm_private { > + struct drm_device *drm; > int num_gpus; > struct etnaviv_gpu *gpu[ETNA_MAX_PIPES]; > gfp_t shm_gfp_mask;