Re: [PATCH v12 01/10] i915: only initialize struct ref_tracker_dir once

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2025-05-30 at 11:01 +0000, Krzysztof Karas wrote:
> Hi Jeff,
> 
> > I got some warnings from the i915 CI with the ref_tracker debugfs
> > patches applied, that indicated that these ref_tracker_dir_init() calls
> > were being called more than once. If references were held on these
> > objects between the initializations, then that could lead to leaked ref
> > tracking objects.
> > 
> > Since these objects are zalloc'ed, ensure that they are only initialized
> > once by testing whether the first byte of the name field is 0.
> 
> Are you referring to these warnings?
> <3> [314.043410] debugfs: File 'intel_wakeref@ffff88815111a308' in directory 'ref_tracker' already present!
> <4> [314.043427] ref_tracker: ref_tracker: unable to create debugfs file for intel_wakeref@ffff88815111a308: -EEXIST
> 
> I think those might be caused by introduction of:
> "ref_tracker: automatically register a file in debugfs for a ref_tracker_dir".
> 
> Current version of "ref_tracker: add a static classname string
> to each ref_tracker_dir" further in this series should prevent
> multiple calls to "ref_tracker_dir_init()", so this patch could
> be dropped I think.
> If my reasoning is wrong however, then please add a note to the
> commit message which explains why this is needed in more detail
> or/and move this patch right before it is necessary. Otherwise
> it looks like a vague workaround.
> 

I'm fine with dropping this patch.

Those are the messages that demonstrate the problem, but the problem is
potentially bigger than those messages. ref_tracker_dir_init() is being
called (at least) twice:

struct ref_tracker_dir {                                              
#ifdef CONFIG_REF_TRACKER                                             
        spinlock_t              lock;                                 
        unsigned int            quarantine_avail;                     
        refcount_t              untracked;                            
        refcount_t              no_tracker;                           
        bool                    dead;                                 
        struct list_head        list; /* List of active trackers */   
        struct list_head        quarantine; /* List of dead trackers */
        const char              *class; /* object classname */        
#ifdef CONFIG_DEBUG_FS                                                
        struct dentry           *dentry;                              
        struct dentry           *symlink;                             
#endif                                                                
#endif                                                                
}; 

This structure contains two list_heads that can contain ref_tracker
objects. If that list was populated when ref_tracker_dir_init() is
called the second time, then those objects will now be sitting on a
corrupt list. At best they'll just leak, but with them sitting on a
now-corrupt list, they could cause a panic too.

It may be that there can be no objects on that list when it's called
the second time. But with this patchset initializing it twice will
cause dentry leaks at least.
-- 
Jeff Layton <jlayton@kernel.org>