From bf8a8acbd5dc4cd737848163e44fd59ece6f9e8b Mon Sep 17 00:00:00 2001 From: Justus Winter <4winter@informatik.uni-hamburg.de> Date: Wed, 14 May 2014 11:19:35 +0200 Subject: [PATCH 13/13] libdiskfs: lock-less reference counting of nodes * libdiskfs/diskfs.h (struct node): Use refcounts_t for reference counting. (diskfs_node_refcnt_lock): Remove. (diskfs_node_norefs,diskfs_drop_node): Change comments accordingly. * libdiskfs/init-init.c: Likewise. * libdiskfs/node-drop.c: Likewise. * libdiskfs/node-make.c: Likewise. * libdiskfs/node-nput.c: Likewise. * libdiskfs/node-nputl.c: Likewise. * libdiskfs/node-nref.c: Likewise. * libdiskfs/node-nrefl.c: Likewise. * libdiskfs/node-nrele.c: Likewise. * libdiskfs/node-nrelel.c: Likewise. * ext2fs/inode.c: Likewise. * fatfs/inode.c: Likewise. * isofs/inode.c: Likewise. * tmpfs/node.c: Likewise. * doc/hurd.texi: Likewise. --- doc/hurd.texi | 11 ++-------- ext2fs/inode.c | 9 +++------ fatfs/inode.c | 21 ++++++------------- isofs/inode.c | 9 +++------ libdiskfs/diskfs.h | 12 ++++------- libdiskfs/init-init.c | 2 -- libdiskfs/node-drop.c | 9 +++------ libdiskfs/node-make.c | 3 +-- libdiskfs/node-nput.c | 54 +++++++++++++++++++------------------------------ libdiskfs/node-nputl.c | 12 ++++------- libdiskfs/node-nref.c | 10 ++++----- libdiskfs/node-nrefl.c | 6 +++--- libdiskfs/node-nrele.c | 48 +++++++++++++++++++++---------------------- libdiskfs/node-nrelel.c | 9 +++------ tmpfs/node.c | 9 +++------ 15 files changed, 83 insertions(+), 141 deletions(-) diff --git a/doc/hurd.texi b/doc/hurd.texi index 07ddfb4..6cafdb9 100644 --- a/doc/hurd.texi +++ b/doc/hurd.texi @@ -3780,10 +3780,6 @@ new thread and (eventually) get rid of the old one; the old thread won't do any more syncs, regardless. @end deftypefun -@deftypevar spin_lock_t diskfs_node_refcnt_lock -Pager reference count lock. -@end deftypevar - @deftypevar int diskfs_readonly Set to zero if the filesystem is currently writable. @end deftypevar @@ -3818,9 +3814,7 @@ Every file or directory is a diskfs @dfn{node}. The following functions help your diskfs callbacks manage nodes and their references: @deftypefun void diskfs_drop_node (@w{struct node *@var{np}}) -Node @var{np} now has no more references; clean all state. The -@var{diskfs_node_refcnt_lock} must be held, and will be released upon -return. @var{np} must be locked. +Node @var{np} now has no more references; clean all state. @end deftypefun @deftypefun void diskfs_node_update (@w{struct node *@var{np}}, @w{int @var{wait}}) @@ -4236,14 +4230,13 @@ without real users. @deftypefun void diskfs_try_dropping_softrefs (@w{struct node *@var{np}}) Node @var{np} has some light references, but has just lost its last hard references. Take steps so that if any light references can be freed, -they are. Both @var{diskfs_node_refcnt_lock} and @var{np} are locked. +they are. @var{np} is locked. This function will be called after @code{diskfs_lost_hardrefs}. @end deftypefun @deftypefun void diskfs_node_norefs (@w{struct node *@var{np}}) Node @var{np} has no more references; free local state, including @code{*@var{np}} if it shouldn't be retained. -@var{diskfs_node_refcnt_lock} is held. @end deftypefun @deftypefun error_t diskfs_set_hypermetadata (@w{int @var{wait}}, @w{int @var{clean}}) diff --git a/ext2fs/inode.c b/ext2fs/inode.c index aa070a2..7ec343f 100644 --- a/ext2fs/inode.c +++ b/ext2fs/inode.c @@ -190,12 +190,9 @@ diskfs_try_dropping_softrefs (struct node *np) { /* 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); - - if (references > 0) + struct references result; + refcounts_references (&np->refcounts, &result); + if (result.hard > 0 || result.weak > 1) { /* A reference was reacquired through a hash table lookup. It's fine, we didn't touch anything yet. */ diff --git a/fatfs/inode.c b/fatfs/inode.c index 8b3385b..d5750b4 100644 --- a/fatfs/inode.c +++ b/fatfs/inode.c @@ -237,14 +237,8 @@ diskfs_node_norefs (struct node *np) if (np->dn->translator) free (np->dn->translator); - /* It is safe to unlock diskfs_node_refcnt_lock here for a while because - all references to the node have been deleted. */ if (np->dn->dirnode) - { - pthread_spin_unlock (&diskfs_node_refcnt_lock); - diskfs_nrele (np->dn->dirnode); - pthread_spin_lock (&diskfs_node_refcnt_lock); - } + diskfs_nrele (np->dn->dirnode); assert (!np->dn->pager); @@ -262,12 +256,9 @@ diskfs_try_dropping_softrefs (struct node *np) { /* 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); - - if (references > 0) + struct references result; + refcounts_references (&np->refcounts, &result); + if (result.hard > 0 || result.weak > 1) { /* A reference was reacquired through a hash table lookup. It's fine, we didn't touch anything yet. */ @@ -372,7 +363,7 @@ read_node (struct node *np, vm_address_t buf) /* Files in fatfs depend on the directory that hold the file. */ np->dn->dirnode = dp; if (dp) - dp->references++; + refcounts_ref (&dp->refcounts, NULL); pthread_rwlock_rdlock (&np->dn->dirent_lock); @@ -814,7 +805,7 @@ diskfs_alloc_node (struct node *dir, mode_t mode, struct node **node) /* FIXME: We know that readnode couldn't put this in. */ np->dn->dirnode = dir; - dir->references++; + refcounts_ref (&dir->refcounts, NULL); *node = np; return 0; diff --git a/isofs/inode.c b/isofs/inode.c index 4f22086..b36a289 100644 --- a/isofs/inode.c +++ b/isofs/inode.c @@ -536,12 +536,9 @@ diskfs_try_dropping_softrefs (struct node *np) /* Check if someone reacquired a reference through the node_cache. */ - unsigned int references; - pthread_spin_lock (&diskfs_node_refcnt_lock); - references = np->references; - pthread_spin_unlock (&diskfs_node_refcnt_lock); - - if (references > 0) + struct references result; + refcounts_references (&np->refcounts, &result); + if (result.hard > 0 || result.weak > 1) { /* A reference was reacquired through a hash table lookup. It's fine, we didn't touch anything yet. */ diff --git a/libdiskfs/diskfs.h b/libdiskfs/diskfs.h index ae1a150..2f5a76c 100644 --- a/libdiskfs/diskfs.h +++ b/libdiskfs/diskfs.h @@ -96,8 +96,7 @@ struct node pthread_mutex_t lock; - int references; /* hard references */ - int light_references; /* light references */ + refcounts_t refcounts; mach_port_t sockaddr; /* address for S_IFSOCK shortcut */ @@ -198,8 +197,6 @@ extern volatile struct mapped_time_value *diskfs_mtime; be done by format independent code. */ extern int diskfs_synchronous; -extern pthread_spinlock_t diskfs_node_refcnt_lock; - extern int pager_port_type; /* Whether the filesystem is currently writable or not. */ @@ -448,7 +445,7 @@ error_t diskfs_alloc_node (struct node *dp, mode_t mode, struct node **np); void diskfs_free_node (struct node *np, mode_t mode); /* Node NP has no more references; free local state, including *NP - if it isn't to be retained. diskfs_node_refcnt_lock is held. */ + if it isn't to be retained. */ void diskfs_node_norefs (struct node *np); /* The user must define this function. Node NP has some light @@ -611,9 +608,8 @@ void diskfs_spawn_first_thread (ports_demuxer_type demuxer); diskfs_init_completed once it has a valid proc and auth port. */ void diskfs_start_bootstrap (); -/* Node NP now has no more references; clean all state. The - _diskfs_node_refcnt_lock must be held, and will be released - upon return. NP must be locked. */ +/* Node NP now has no more references; clean all state. NP must be + locked. */ void diskfs_drop_node (struct node *np); /* Set on disk fields from NP->dn_stat; update ctime, atime, and mtime diff --git a/libdiskfs/init-init.c b/libdiskfs/init-init.c index 35be7ed..976b4e8 100644 --- a/libdiskfs/init-init.c +++ b/libdiskfs/init-init.c @@ -37,8 +37,6 @@ int _diskfs_noatime; struct hurd_port _diskfs_exec_portcell; -pthread_spinlock_t diskfs_node_refcnt_lock = PTHREAD_SPINLOCK_INITIALIZER; - pthread_spinlock_t _diskfs_control_lock = PTHREAD_SPINLOCK_INITIALIZER; int _diskfs_ncontrol_ports; diff --git a/libdiskfs/node-drop.c b/libdiskfs/node-drop.c index 83eb590..fab3cfa 100644 --- a/libdiskfs/node-drop.c +++ b/libdiskfs/node-drop.c @@ -31,9 +31,8 @@ free_modreqs (struct modreq *mr) } -/* Node NP now has no more references; clean all state. The - diskfs_node_refcnt_lock must be held, and will be released - upon return. NP must be locked. */ +/* Node NP now has no more references; clean all state. NP must be + locked. */ void diskfs_drop_node (struct node *np) { @@ -60,8 +59,7 @@ diskfs_drop_node (struct node *np) and an nput. The next time through, this routine will notice that the size is zero, and not have to do anything. */ - np->references++; - pthread_spin_unlock (&diskfs_node_refcnt_lock); + refcounts_ref (&np->refcounts, NULL); diskfs_truncate (np, 0); /* Force allocsize to zero; if truncate consistently fails this @@ -94,5 +92,4 @@ diskfs_drop_node (struct node *np) assert (!np->sockaddr); diskfs_node_norefs (np); - pthread_spin_unlock (&diskfs_node_refcnt_lock); } diff --git a/libdiskfs/node-make.c b/libdiskfs/node-make.c index 2b6ef2a..fc4f54c 100644 --- a/libdiskfs/node-make.c +++ b/libdiskfs/node-make.c @@ -36,8 +36,7 @@ diskfs_make_node (struct disknode *dn) np->dn_stat_dirty = 0; pthread_mutex_init (&np->lock, NULL); - np->references = 1; - np->light_references = 0; + refcounts_init (&np->refcounts, 1, 0); np->owner = 0; np->sockaddr = MACH_PORT_NULL; diff --git a/libdiskfs/node-nput.c b/libdiskfs/node-nput.c index 5043ad1..2935ae2 100644 --- a/libdiskfs/node-nput.c +++ b/libdiskfs/node-nput.c @@ -26,56 +26,44 @@ void diskfs_nput (struct node *np) { - int tried_drop_softrefs = 0; + struct references result; - loop: - pthread_spin_lock (&diskfs_node_refcnt_lock); - assert (np->references); - np->references--; - if (np->references + np->light_references == 0) - diskfs_drop_node (np); - else if (np->references == 0 && !tried_drop_softrefs) - { - pthread_spin_unlock (&diskfs_node_refcnt_lock); + /* While we call the diskfs_try_dropping_softrefs, we need to hold + one reference. We use a weak reference for this purpose, which + we acquire by demoting our hard reference to a weak one. */ + refcounts_demote (&np->refcounts, &result); + if (result.hard == 0) + { /* This is our cue that something akin to "last process closes file" in the POSIX.1 sense happened, so make sure any pending node time updates now happen in a timely fashion. */ diskfs_set_node_times (np); - diskfs_lost_hardrefs (np); if (!np->dn_stat.st_nlink) { - /* There are no links. If there are soft references that - can be dropped, we can't let them postpone deallocation. - So attempt to drop them. But that's a user-supplied - routine, which might result in further recursive calls to - the ref-counting system. So we have to reacquire our - reference around the call to forestall disaster. */ - pthread_spin_lock (&diskfs_node_refcnt_lock); - np->references++; - pthread_spin_unlock (&diskfs_node_refcnt_lock); - if (np->sockaddr != MACH_PORT_NULL) { mach_port_deallocate (mach_task_self (), np->sockaddr); np->sockaddr = MACH_PORT_NULL; } + /* There are no links. If there are soft references that + can be dropped, we can't let them postpone deallocation. + So attempt to drop them. But that's a user-supplied + routine, which might result in further recursive calls to + the ref-counting system. This is not a problem, as we + hold a weak reference ourselves. */ diskfs_try_dropping_softrefs (np); - - /* But there's no value in looping forever in this - routine; only try to drop soft refs once. */ - tried_drop_softrefs = 1; - - /* Now we can drop the reference back... */ - goto loop; } pthread_mutex_unlock (&np->lock); } - else - { - pthread_spin_unlock (&diskfs_node_refcnt_lock); - pthread_mutex_unlock (&np->lock); - } + + /* Finally get rid of our reference. */ + refcounts_deref_weak (&np->refcounts, &result); + + if (result.hard == 0 && result.weak == 0) + diskfs_drop_node (np); + + pthread_mutex_unlock (&np->lock); } diff --git a/libdiskfs/node-nputl.c b/libdiskfs/node-nputl.c index 1959665..8dac16e 100644 --- a/libdiskfs/node-nputl.c +++ b/libdiskfs/node-nputl.c @@ -25,14 +25,10 @@ void diskfs_nput_light (struct node *np) { - pthread_spin_lock (&diskfs_node_refcnt_lock); - assert (np->light_references); - np->light_references--; - if (np->references + np->light_references == 0) + struct references result; + refcounts_deref_weak (&np->refcounts, &result); + if (result.hard == 0 && result.weak == 0) diskfs_drop_node (np); else - { - pthread_spin_unlock (&diskfs_node_refcnt_lock); - pthread_mutex_unlock (&np->lock); - } + pthread_mutex_unlock (&np->lock); } diff --git a/libdiskfs/node-nref.c b/libdiskfs/node-nref.c index 13cea05..89ffa4f 100644 --- a/libdiskfs/node-nref.c +++ b/libdiskfs/node-nref.c @@ -26,12 +26,10 @@ void diskfs_nref (struct node *np) { - int new_hardref; - pthread_spin_lock (&diskfs_node_refcnt_lock); - np->references++; - new_hardref = (np->references == 1); - pthread_spin_unlock (&diskfs_node_refcnt_lock); - if (new_hardref) + struct references result; + refcounts_ref (&np->refcounts, &result); + assert (result.hard > 1 || result.weak > 0); + if (result.hard == 1) { pthread_mutex_lock (&np->lock); diskfs_new_hardrefs (np); diff --git a/libdiskfs/node-nrefl.c b/libdiskfs/node-nrefl.c index 9692247..b7af409 100644 --- a/libdiskfs/node-nrefl.c +++ b/libdiskfs/node-nrefl.c @@ -24,7 +24,7 @@ void diskfs_nref_light (struct node *np) { - pthread_spin_lock (&diskfs_node_refcnt_lock); - np->light_references++; - pthread_spin_unlock (&diskfs_node_refcnt_lock); + struct references result; + refcounts_ref_weak (&np->refcounts, &result); + assert (result.hard > 0 || result.weak > 1); } diff --git a/libdiskfs/node-nrele.c b/libdiskfs/node-nrele.c index cc68089..d962846 100644 --- a/libdiskfs/node-nrele.c +++ b/libdiskfs/node-nrele.c @@ -28,38 +28,36 @@ void diskfs_nrele (struct node *np) { - int tried_drop_softrefs = 0; + struct references result; - loop: - pthread_spin_lock (&diskfs_node_refcnt_lock); - assert (np->references); - np->references--; - if (np->references + np->light_references == 0) - { - pthread_mutex_lock (&np->lock); - diskfs_drop_node (np); - } - else if (np->references == 0) + /* While we call the diskfs_try_dropping_softrefs, we need to hold + one reference. We use a weak reference for this purpose, which + we acquire by demoting our hard reference to a weak one. */ + refcounts_demote (&np->refcounts, &result); + + if (result.hard == 0) { pthread_mutex_lock (&np->lock); - pthread_spin_unlock (&diskfs_node_refcnt_lock); diskfs_lost_hardrefs (np); - if (!np->dn_stat.st_nlink && !tried_drop_softrefs) + if (!np->dn_stat.st_nlink) { - /* Same issue here as in nput; see that for explanation */ - pthread_spin_lock (&diskfs_node_refcnt_lock); - np->references++; - pthread_spin_unlock (&diskfs_node_refcnt_lock); - + /* There are no links. If there are soft references that + can be dropped, we can't let them postpone deallocation. + So attempt to drop them. But that's a user-supplied + routine, which might result in further recursive calls to + the ref-counting system. This is not a problem, as we + hold a weak reference ourselves. */ diskfs_try_dropping_softrefs (np); - tried_drop_softrefs = 1; - - /* Now we can drop the reference back... */ - pthread_mutex_unlock (&np->lock); - goto loop; } pthread_mutex_unlock (&np->lock); } - else - pthread_spin_unlock (&diskfs_node_refcnt_lock); + + /* Finally get rid of our reference. */ + refcounts_deref_weak (&np->refcounts, &result); + + if (result.hard == 0 && result.weak == 0) + { + pthread_mutex_lock (&np->lock); + diskfs_drop_node (np); + } } diff --git a/libdiskfs/node-nrelel.c b/libdiskfs/node-nrelel.c index ee53b22..dc4f920 100644 --- a/libdiskfs/node-nrelel.c +++ b/libdiskfs/node-nrelel.c @@ -26,14 +26,11 @@ void diskfs_nrele_light (struct node *np) { - pthread_spin_lock (&diskfs_node_refcnt_lock); - assert (np->light_references); - np->light_references--; - if (np->references + np->light_references == 0) + struct references result; + refcounts_deref_weak (&np->refcounts, &result); + if (result.hard == 0 && result.weak == 0) { pthread_mutex_lock (&np->lock); diskfs_drop_node (np); } - else - pthread_spin_unlock (&diskfs_node_refcnt_lock); } diff --git a/tmpfs/node.c b/tmpfs/node.c index 74971ec..275bb3c 100644 --- a/tmpfs/node.c +++ b/tmpfs/node.c @@ -285,12 +285,9 @@ diskfs_try_dropping_softrefs (struct node *np) { /* Check if someone reacquired a reference through the all_nodes. */ - unsigned int references; - pthread_spin_lock (&diskfs_node_refcnt_lock); - references = np->references; - pthread_spin_unlock (&diskfs_node_refcnt_lock); - - if (references > 0) + struct references result; + refcounts_references (&np->refcounts, &result); + if (result.hard > 0 || result.weak > 1) { /* A reference was reacquired through a hash table lookup. It's fine, we didn't touch anything yet. */ -- 2.0.0.rc0