Re: [PATCH v9 08/12] device core: Introduce DMA range map, supplanting dma_pfn_offset

Nicolas Saenz Julienne
2020-07-30T17:02:52Z

On Thu, 2020-07-30 at 12:44 -0400, Jim Quinlan wrote:
> On Wed, Jul 29, 2020 at 10:28 AM Rob Herring <robh+dt@kernel.org> wrote:
> > On Wed, Jul 29, 2020 at 12:19 AM Christoph Hellwig <hch@lst.de> wrote:
> > > On Tue, Jul 28, 2020 at 02:24:51PM -0400, Jim Quinlan wrote:
> > > > I started using devm_kcalloc() but at least two reviewers convinced me
> > > > to just use kcalloc(). In addition, when I was using devm_kcalloc()
> > > > it was awkward because 'dev' is not available to this function.
> > > >
> > > > It comes down to whether unbind/binding the device N times is actually
> > > > a reasonable usage. As for my experience I've seen two cases: (1) my
> > > > overnight "bind/unbind the PCIe RC driver" script, and we have a
> > > > customer who does an unbind/bind as a hail mary to bring back life to
> > > > their dead EP device. If the latter case happens repeatedly, there
> > > > are bigger problems.
> > >
> > > We can't just leak the allocations. Do you have a pointer to the
> > > arguments against managed resources? I'm generally not a huge fan
> > > of the managed resources, but for a case like this they actually seem
> > > useful. If we don't use the managed resources we'll at leat need
> > > to explicitly free the resources when freeing the device.
> >
> > The lifetime for devm_kcalloc may not be what we want here. devm
> > allocs are freed on probe fail or remove, not on freeing the device
> > (there is a just in case free there too though).
>
> What do you suggest doing as an alternative?

I haven't given the idea much thought, but how about using a kref in the first
element of your bus_dma_region array. It should be increased upon assigning it
to a device and decreased it upon destroying it (triggering the free once it
hits 0). It would take care of this memory leak, but also useful where you're
sharing bus_dma_regions between devices.

Regards,
Nicolas