[PATCH] sched/wake_q: Restore task_struct::wake_q type safety

[Ingo Molnar]

* Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Wed, Feb 8, 2017 at 10:34 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> > To be able to decouple wake_q functionality from <linux/sched.h> make
> > the basic task_struct::wake_q pointer an opaque void *.
> 
> Please don't use "void *" for opaque pointers.
> 
> You lose all typechecking, and this is just complete garbage:
> 
> +       struct wake_q_node *node = (void *)&task->wake_q;
> 
> What? You're casting a "void *" to "void *"? WTF?

Well, I'm casting 'void **' to 'void *', so the cast is necessary in this specific 
scheme - but yeah, this is still pretty ugly.

> The proper way to do opaque pointers is to declare them as pointers to
> a structure that hasn't been defined (ie use a "struct xyz;" forward
> declaration), and then that structure is only actually defined in the
> places that use the otherwise opaque pointer.
> 
> That way you
> 
>  (a) never need to cast anything
> 
>  (b) get proper (and strong) type checking for the pointers.
> 
> and the people who need to look into it automatically do the right
> thing, while anybody else who tries to dereference or otherwise use
> the struct pointer will get a compiler error.

So the problem here is that we don't really want an opaque pointer in a classic 
sense, but an opaque field.

struct wake_q is:

  struct wake_q_node {
          struct wake_q_node *next;
  };

i.e. it's really just a single linked list node, a single pointer. This line:

> +       struct wake_q_node *node = (void *)&task->wake_q;

gets the pointer to the list node.

Couldn't think of a nicer way to do this - so let's not do this at all: we can 
just declare struct wake_q_node in sched.h and be done with it. We saved thousands 
of lines of code from it already, it's not worth doing it at the cost of strong 
type safety.

I.e. something like the patch attached below on top of WIP.sched/core?

Untested.

Thanks,

	Ingo


==============================>