On Tue, Feb 11, 2025 at 01:14:28AM +0200, Dmitry Baryshkov wrote:
> 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?
No, %px must not be used there. %p is mangled but should be consistent
across calls. But yeah, it's kind of the reason I suggested to use the
bridge device name instead.
Maxime
Attachment:
signature.asc
Description: PGP signature
