Re: [RFC V2] kthread: add object debug support

[Stephen Boyd]

Quoting Thomas Gleixner (2020-08-12 03:27:03)
> Stephen,
> 
> Stephen Boyd <sboyd@xxxxxxxxxx> writes:
> > Quoting Qianli Zhao (2020-08-11 22:14:14)
> >> +/********** kernel/kthread **********/
> >> +#define KWORK_ENTRY_STATIC     ((void *) 0x600 + POISON_POINTER_DELTA)
> >
> > Is this related to the debugobjects change here? It looks like another
> > version of list poison.
> 
> Yes, it is. We use these poison entries to mark statically allocated
> objects. debug objects does not know about statically initialized
> objects up to the point where they are used (activated).
> 
> That means the object state lookup will fail which causes debugobjects
> to complain about using an uninitialized object. But in case of static
> initialized ones that's a false positive. So we mark these objects in
> their list head (or some other appropriate place) with a poison value
> and in case of a failed lookup debug object does:
> 
>         if (descr->is_static_object && descr->is_static_object(addr)) {
>                 /* track this static object */
>                 debug_object_init(addr, descr);
>                 debug_object_activate(addr, descr);
>         }        
> 
> The object specific is_static_object() callback will then check for the
> magic list poison value being present:

Thanks! I missed this function below.

> 
> > +static bool kwork_is_static_object(void *addr)
> > +{
> > +     struct kthread_work *kwork = addr;
> > +
> > +     return (kwork->node.prev == NULL &&
> > +             kwork->node.next == KWORK_ENTRY_STATIC);
> > +}
> 
> and if so the debug object core fixes its internal state by creating a
> tracking object and then activating it.
> 
> It's not a perfect "yes this is statically initialized" check but good
> enough. If you go and do:
> 
>    work = kzalloc(sizeof(*work);
>    work->node.next = KWORK_ENTRY_STATIC;
> 
>    kthread_insert_work(worker, work);
> 
> or any other variant of insanity which makes the check claim that this
> is statically initialized then you rightfully can keep the pieces :)

Makes sense. Maybe this "technique" should be documented in
Documentation/core-api/debug-objects.rst? I can cook up a patch to add
is_static_object() to the Fixup functions section.