Quoting Samuel Thibault (2014-06-02 03:41:55)
> Justus Winter, le Sun 01 Jun 2014 22:03:01 +0200, a écrit :
> > @@ -46,8 +46,18 @@
> > #define INOHASH(ino) (((unsigned)(ino))%INOHSZ)
> > #endif
> >
> > +/* The nodehash is a cache of nodes.
> > +
> > + Access to nodehash and nodehash_nr_items is protected by
> > + nodecache_lock.
> > +
> > + Every node in the nodehash carries a light reference. When we are
> > + asked to give up that light reference, we reacquire our lock
> > + momentarily to check whether someone else reacquired a reference
> > + through the nodehash. */
> > static struct node *nodehash[INOHSZ];
> > static size_t nodehash_nr_items;
> > +static pthread_rwlock_t nodecache_lock = PTHREAD_RWLOCK_INITIALIZER;
>
> Please also document that this is to be taken before
> diskfs_node_refcnt_lock.
Actually, the whole point of this excercise is to get rid of
diskfs_node_refcnt_lock.
>
> > @@ -71,24 +81,22 @@ diskfs_cached_lookup (ino_t inum, struct node **npp)
> > struct node *np;
> > struct disknode *dn;
> >
> > - pthread_spin_lock (&diskfs_node_refcnt_lock);
> > + pthread_rwlock_rdlock (&nodecache_lock);
> > for (np = nodehash[INOHASH(inum)]; np; np = np->dn->hnext)
> > if (np->cache_id == inum)
> > {
> > - np->references++;
> > - pthread_spin_unlock (&diskfs_node_refcnt_lock);
> > + diskfs_nref (np);
> > + pthread_rwlock_unlock (&nodecache_lock);
> > pthread_mutex_lock (&np->lock);
> > *npp = np;
> > return 0;
> > }
> > + pthread_rwlock_unlock (&nodecache_lock);
>
> But what if two threads call diskfs_cached_lookup for the same node at
> the same time? They'll both think it's not in the hash yet, and will
> both create the node and add it to the hash. So...
>
> > /* Format specific data for the new node. */
> > dn = malloc (sizeof (struct disknode));
> > if (! dn)
> > - {
> > - pthread_spin_unlock (&diskfs_node_refcnt_lock);
> > - return ENOMEM;
> > - }
> > + return ENOMEM;
> > dn->dirents = 0;
> > dn->dir_idx = 0;
> > dn->pager = 0;
> > @@ -102,14 +110,15 @@ diskfs_cached_lookup (ino_t inum, struct node **npp)
> > pthread_mutex_lock (&np->lock);
> >
> > /* Put NP in NODEHASH. */
> > + pthread_rwlock_wrlock (&nodecache_lock);
>
> ... after taking the pthread_rwlock_wrlock again in wr mode, you need to
> check for the existence of the node again.
Good point...
Justus
>
> > dn->hnext = nodehash[INOHASH(inum)];
> > if (dn->hnext)
> > dn->hnext->dn->hprevp = &dn->hnext;
> > dn->hprevp = &nodehash[INOHASH(inum)];
> > nodehash[INOHASH(inum)] = np;
> > + diskfs_nref_light (np);
> > nodehash_nr_items += 1;
> > -
> > - pthread_spin_unlock (&diskfs_node_refcnt_lock);
> > + pthread_rwlock_unlock (&nodecache_lock);
> >
> > /* Get the contents of NP off disk. */
> > err = read_node (np);
>
> Samuel
