Re: [PATCH v7 3/7] drm/i915: Prepare for multiple GTs

Andi Shyti
2022-03-20T15:52:10Z

Hi Michal,

[...]

> > +static int intel_gt_tile_setup(struct intel_gt *gt, phys_addr_t phys_addr)
> > +{
> > + int ret;
> > +
> > + if (!gt_is_root(gt)) {
> > + struct intel_uncore_mmio_debug *mmio_debug;
> > + struct intel_uncore *uncore;
> > +
> > + uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);
> > + if (!uncore)
> > + return -ENOMEM;
> > +
> > + mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);
> > + if (!mmio_debug) {
> > + kfree(uncore);
> > + return -ENOMEM;
> > + }
> > +
> > + gt->uncore = uncore;
> > + gt->uncore->debug = mmio_debug;
> > +
> > + __intel_gt_init_early(gt);
> > + }
> > +
> > + intel_uncore_init_early(gt->uncore, gt);
> > +
> > + ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
> > + if (ret)
> > + return ret;
>
> (little guessing as in this patch we don't have non-root gt yet)
>
> if the future, when we will be doing setup of non-root gt, if we return
> here then likely we will leak both uncore/mmio_debug as gt will not be
> added to i915->gts thus it will not be visible in for_each_gt loop used
> to release/cleanup all gts.
>
> since in above code you are doing cleanup in case of kzalloc failure,
> same should be done in case of mmio setup failure.

that's a good point. In the next patch I am going to add support
for the first multitile platform and, because it's too old to
remember, I had a look and I think this part is not properly
managed.

Thanks for the note!

Andi