From e17366f09a6003dcc95633ce312aa3753f056e28 Mon Sep 17 00:00:00 2001 From: Justus Winter <4winter@informatik.uni-hamburg.de> Date: Tue, 13 May 2014 15:14:53 +0200 Subject: fatfs: use a seperate lock to protect nodehash Previously, fatfs used diskfs_node_refcnt_lock to serialize access to the nodehash. Use a separate lock to protect nodehash. Adjust the reference counting accordingly. 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. * fatfs/inode.c (nodecache_lock): New lock. (diskfs_cached_lookup): Use a separate lock to protect nodehash. Adjust the reference counting accordingly. (ifind): Likewise. (diskfs_node_iterate): Likewise. (diskfs_node_norefs): Move the code removing the node from nodehash... (diskfs_try_dropping_softrefs): ... here, where we check whether someone reacquired a reference, and if so hold on to our light reference. --- fatfs/inode.c | 147 ++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 102 insertions(+), 45 deletions(-) (limited to 'fatfs') diff --git a/fatfs/inode.c b/fatfs/inode.c index ed6f3f08..1d670f53 100644 --- a/fatfs/inode.c +++ b/fatfs/inode.c @@ -44,8 +44,19 @@ #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; +/* nodecache_lock must be acquired before diskfs_node_refcnt_lock. */ +static pthread_rwlock_t nodecache_lock = PTHREAD_RWLOCK_INITIALIZER; static error_t read_node (struct node *np, vm_address_t buf); @@ -58,33 +69,38 @@ inode_init () nodehash[n] = 0; } +/* Lookup node with inode number INUM. Returns NULL if the node is + not found in the node cache. */ +static struct node * +lookup (ino_t inum) +{ + struct node *np; + for (np = nodehash[INOHASH(inum)]; np; np = np->dn->hnext) + if (np->cache_id == inum) + return np; + return NULL; +} + /* Fetch inode INUM, set *NPP to the node structure; gain one user reference and lock the node. */ error_t diskfs_cached_lookup (ino64_t inum, struct node **npp) { error_t err; - struct node *np; + struct node *np, *tmp; struct disknode *dn; - pthread_spin_lock (&diskfs_node_refcnt_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); - pthread_mutex_lock (&np->lock); - *npp = np; - return 0; - } + pthread_rwlock_rdlock (&nodecache_lock); + np = lookup (inum); + if (np) + goto gotit; + pthread_rwlock_unlock (&nodecache_lock); /* 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->pager = 0; dn->first = 0; dn->last = 0; @@ -102,15 +118,25 @@ diskfs_cached_lookup (ino64_t inum, struct node **npp) pthread_mutex_lock (&np->lock); /* Put NP in NODEHASH. */ + pthread_rwlock_wrlock (&nodecache_lock); + tmp = lookup (inum); + if (tmp) + { + /* We lost a race. */ + diskfs_nput (np); + np = tmp; + goto gotit; + } + 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_rwlock_unlock (&nodecache_lock); - pthread_spin_unlock (&diskfs_node_refcnt_lock); - /* Get the contents of NP off disk. */ err = read_node (np, 0); @@ -121,6 +147,13 @@ diskfs_cached_lookup (ino64_t inum, struct node **npp) *npp = np; return 0; } + + gotit: + diskfs_nref (np); + pthread_rwlock_unlock (&nodecache_lock); + pthread_mutex_lock (&np->lock); + *npp = np; + return 0; } /* Fetch inode INUM, set *NPP to the node structure; @@ -133,24 +166,23 @@ diskfs_cached_lookup_in_dirbuf (int inum, struct node **npp, vm_address_t buf) 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); /* 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->pager = 0; dn->first = 0; dn->last = 0; @@ -168,15 +200,16 @@ diskfs_cached_lookup_in_dirbuf (int inum, struct node **npp, vm_address_t buf) pthread_mutex_lock (&np->lock); /* Put NP in NODEHASH. */ + pthread_rwlock_wrlock (&nodecache_lock); 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_rwlock_unlock (&nodecache_lock); - pthread_spin_unlock (&diskfs_node_refcnt_lock); - /* Get the contents of NP off disk. */ err = read_node (np, buf); @@ -196,17 +229,12 @@ ifind (ino_t inum) { struct node *np; - pthread_spin_lock (&diskfs_node_refcnt_lock); - for (np = nodehash[INOHASH(inum)]; np; np = np->dn->hnext) - { - if (np->cache_id != inum) - continue; + pthread_rwlock_rdlock (&nodecache_lock); + np = lookup (inum); + pthread_rwlock_unlock (&nodecache_lock); - assert (np->references); - pthread_spin_unlock (&diskfs_node_refcnt_lock); - return np; - } - assert (0); + assert (np); + return np; } /* The last reference to a node has gone away; drop it from the hash @@ -216,11 +244,6 @@ diskfs_node_norefs (struct node *np) { struct cluster_chain *last = np->dn->first; - *np->dn->hprevp = np->dn->hnext; - if (np->dn->hnext) - np->dn->hnext->dn->hprevp = np->dn->hprevp; - nodehash_nr_items -= 1; - while (last) { struct cluster_chain *next = last->next; @@ -251,6 +274,36 @@ diskfs_node_norefs (struct node *np) void diskfs_try_dropping_softrefs (struct node *np) { + pthread_rwlock_wrlock (&nodecache_lock); + if (np->dn->hprevp != NULL) + { + /* Check if someone reacquired a reference through the + nodehash. */ + unsigned int references; + pthread_spin_lock (&diskfs_node_refcnt_lock); + references = np->references; + pthread_spin_unlock (&diskfs_node_refcnt_lock); + + /* An additional reference is acquired by libdiskfs across calls + to diskfs_try_dropping_softrefs. */ + if (references > 1) + { + /* A reference was reacquired through a hash table lookup. + It's fine, we didn't touch anything yet. */ + pthread_rwlock_unlock (&nodecache_lock); + return; + } + + *np->dn->hprevp = np->dn->hnext; + if (np->dn->hnext) + np->dn->hnext->dn->hprevp = np->dn->hprevp; + np->dn->hnext = NULL; + np->dn->hprevp = NULL; + nodehash_nr_items -= 1; + diskfs_nrele_light (np); + } + pthread_rwlock_unlock (&nodecache_lock); + drop_pager_softrefs (np); } @@ -554,12 +607,12 @@ diskfs_node_iterate (error_t (*fun)(struct node *)) size_t num_nodes; struct node *node, **node_list, **p; - pthread_spin_lock (&diskfs_node_refcnt_lock); + pthread_rwlock_rdlock (&nodecache_lock); /* We must copy everything from the hash table into another data structure to avoid running into any problems with the hash-table being modified during processing (normally we delegate access to hash-table with - diskfs_node_refcnt_lock, but we can't hold this while locking the + nodecache_lock, but we can't hold this while locking the individual node locks). */ num_nodes = nodehash_nr_items; @@ -570,10 +623,14 @@ diskfs_node_iterate (error_t (*fun)(struct node *)) for (node = nodehash[n]; node; node = node->dn->hnext) { *p++ = node; - node->references++; + + /* We acquire a hard reference for node, but without using + diskfs_nref. We do this so that diskfs_new_hardrefs will not + get called. */ + node->references++; } - pthread_spin_unlock (&diskfs_node_refcnt_lock); + pthread_rwlock_unlock (&nodecache_lock); p = node_list; while (num_nodes-- > 0) -- cgit v1.2.3