On Mon, Feb 10, 2025 at 06:12:44PM +0100, Luca Ceresoli wrote:
> Hi Maxime, Dmitry
>
> On Fri, 7 Feb 2025 21:54:06 +0200
> Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> > > > +/* Internal function (for refcounted bridges) */
> > > > +void __drm_bridge_free(struct kref *kref)
> > > > +{
> > > > + struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
> > > > + void *container = ((void *)bridge) - bridge->container_offset;
> > > > +
> > > > + DRM_DEBUG("bridge=%p, container=%p FREE\n", bridge, container);
> > >
> > > Pointers are not really useful to track here, since they are obfuscated
> > > most of the time. Using the bridge device name would probably be better
> > > (or removing the SHOUTING DEBUG entirely :))
> >
> > bridge device name or bridge funcs (I opted for the latter for the
> > debugfs file)
>
> These DRM_DEBUG()s proved extremely useful exactly because of the
> pointer. This is because when using hotplug one normally has the same
> device added and removed multiple times, and so the device name or
> bridge funcs is always the same, preventing from understanding which
> instance is leaking, or being freed, get, put, etc.
>
> Do you think this is a sufficient motivation to keep it?
Then it should be something like %px. I found that %p is mangled.
What about having both device name _and_ a pointer?
>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
--
With best wishes
Dmitry
