Re: [PATCH] Remove global netfs lock and use hard and soft references for nodes.

[Justus Winter]

Hi,

nitpick: Your subject is still quite long, and you got a newline in
your subject, however you managed to do that o_O

Quoting Flavio Cruz (2016-03-07 00:14:52)
> -  /* The number of references to this node.  */
> -  int references;
> +  /* Hard and soft references to this node.  */
> +  refcounts_t refcounts;

I guess having both hard and soft references allowed you to simplify
some users of netfs?

(Answering myself: Yes, console and fakeroot benefit greatly.)

>  void
>  netfs_nref (struct node *np)
>  {
> -  pthread_spin_lock (&netfs_node_refcnt_lock);
> -  np->references++;
> -  pthread_spin_unlock (&netfs_node_refcnt_lock);
> +  struct references result;
> +  refcounts_ref (&np->refcounts, &result);
> +}

If you are not interested in the result, pass NULL here.

> +void
> +netfs_nref_light (struct node *np)
> +{
> +  struct references result;
> +  refcounts_ref_weak (&np->refcounts, &result);
>  }

Likewise.

> --- a/trans/fakeroot.c
> +++ b/trans/fakeroot.c
> @@ -110,6 +110,9 @@ new_node (file_t file, mach_port_t idport, int locked, 
> int openmodes,
>  
>    if (!locked)
>      pthread_mutex_lock (&idport_ihash_lock);
> +  /* The light reference allows us to safely keep the node in the
> +     hash table.  */
> +  netfs_nref_light (*np);
>    err = hurd_ihash_add (&idport_ihash, nn->idport, *np);
>    if (err)
>      goto lose;
> @@ -155,22 +158,31 @@ set_faked_attribute (struct node *np, unsigned int 
> faked)
>      }
>  }
>  
> +void
> +netfs_try_dropping_softrefs (struct node *np)
> +{
> +  /* We have to drop our light reference by removing the node from the
> +     idport_ihash hash table.  */
> +  pthread_mutex_lock (&idport_ihash_lock);
> +
> +  hurd_ihash_locp_remove (&idport_ihash, netfs_node_netnode 
> (np)->idport_locp);
> +  netfs_nrele_light (np);
> +
> +  pthread_mutex_unlock (&idport_ihash_lock);

I haven't thought this through, but I guess you want to do the
netfs_nrele_light with the idport_ihash_lock unlocked.

Looks fine otherwise,  I'll test it in my builds.

Thanks,
Justus