Re: [PATCH v14] kern: simple futex for gnumach

[Diego Nieto Cid]

Hi Marin,

If you decide to keep working on this, take a look at the following notes

2014/1/13 Marin Ramesa <m...@hi.t-com.hr>:
> +
> +static int futex_shared_init(vm_offset_t address, struct futex **futex)
> +{
> +       vm_map_version_t version;
> +       vm_prot_t prot;
> +       boolean_t wired;
> +
> +       *futex = (struct futex *)kalloc(sizeof(**futex));
> +       if (*futex == NULL)
> +               return -1;
> +
> +       struct rbtree_node node;
> +
> +       kern_return_t kr;
> +       kr = vm_map_lookup(&current_map(), address, VM_PROT_READ,
> +                       &version, &(*futex)->object, &(*futex)->offset, 
> &prot, &wired);
> +       assert(kr == KERN_SUCCESS);
> +
> +       (*futex)->nr_refs = 0;
> +
> +       simple_lock(&futex_shared_lock);
> +
> +       rbtree_insert(&futex_shared_tree, &node, futex_shared_cmp_insert);
> +
> +       (*futex)->node = &node;

You are taking the address of a stack allocated variable and storing
it in a long-lived object. This is wrong for the address is only valid
inside the containing block of the variable it's pointing to.

> +void futex_wait(task_t task, vm_offset_t futex_address, vm_offset_t 
> compare_address, int msec, boolean_t private_futex)
> +{
[snip]
> +               if (*(int *)futex_address == *(int *)compare_address) {
> +
> +                       simple_lock(&futex_shared_lock);
> +
> +                       if (node != NULL)
> +                               futex = rbtree_entry(node, struct futex, 
> node);

I'm not familiarized with the rbtree interface. But after giving a
quick glance at rbtree_entry's definition I think you can't do that
because the rbtree_node structure is not part of the futex structure.

Study how it's used, for instance, in "struct vm_map_entry" declared
in "vm/vm_map.h".