Adding start/stop journal transaction in a few places previously missed.
The diskfs_journal_start_transaction may wait for a commit to drain the
previous transaction. That wait is safe only
when the caller holds no lock a participant of the draining transaction
could need. RPC handlers already take their handle first and their node
locks second. A few paths reached a start with a lock already held:
- diskfs_release_peropen, from the ports clean routine when a client
closes its last port to a file. It locks the node and calls
diskfs_nput; for an unlinked file _diskfs_lastref then writes the
node back under nodecache_lock, and diskfs_drop_node truncates and
frees it under np->lock, each starting a transaction. A participant
blocked on nodecache_lock in diskfs_cached_lookup, or on np->lock
after reacquiring the node through the hash, would wait on this
thread while this thread waited on the drain.
- diskfs_S_fsys_getfile, whose lookup locks the node and whose nput can
drop it.
- diskfs_S_io_read and diskfs_S_io_stat, which call diskfs_node_update
under np->lock for the access time.
- pager_unlock_page, whose block allocation dirties bitmaps, group
descriptors and indirect blocks through several nested starts. The
pager thread never waits, but without one handle around the loop
each allocation could land in a different transaction.
---
ext2fs/pager.c | 9 ++++++---
libdiskfs/fsys-getfile.c | 12 ++++++++++++
libdiskfs/io-read.c | 7 +++++++
libdiskfs/io-stat.c | 7 +++++++
libdiskfs/node-drop.c | 6 ++++++
libdiskfs/peropen-rele.c | 13 +++++++++++++
6 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/ext2fs/pager.c b/ext2fs/pager.c
index 42941150e..2b3fb574d 100644
--- a/ext2fs/pager.c
+++ b/ext2fs/pager.c
@@ -743,9 +743,12 @@ pager_unlock_page (struct user_pager_info *pager,
vm_offset_t page)
struct node *node = pager->node;
struct disknode *dn = diskfs_node_disknode (node);
- /* Block allocation below starts a journal transaction. Mark this
- thread so that start never waits on a draining commit. */
+ /* Block allocation below dirties bitmaps, group descriptors and
+ indirect blocks. Open one handle here so every block this page
+ needs lands in one transaction. Mark this
+ thread so that start never waits on a draining commit. */
journal_thread_set_pager (1);
+ diskfs_transaction_t *txn = diskfs_journal_start_transaction ();
pthread_rwlock_wrlock (&dn->alloc_lock);
@@ -790,7 +793,7 @@ pager_unlock_page (struct user_pager_info *pager,
vm_offset_t page)
STAT_INC (file_page_unlocks);
pthread_rwlock_unlock (&dn->alloc_lock);
-
+ diskfs_journal_stop_transaction (txn);
journal_thread_set_pager (0);
if (err == ENOSPC)
diff --git a/libdiskfs/fsys-getfile.c b/libdiskfs/fsys-getfile.c
index b80c2869f..7affd44bb 100644
--- a/libdiskfs/fsys-getfile.c
+++ b/libdiskfs/fsys-getfile.c
@@ -41,6 +41,7 @@ diskfs_S_fsys_getfile (struct diskfs_control *pt,
struct protid *new_cred;
struct peropen *new_po;
struct iouser *user;
+ diskfs_transaction_t *txn;
if (!pt)
return EOPNOTSUPP;
@@ -52,15 +53,21 @@ diskfs_S_fsys_getfile (struct diskfs_control *pt,
f = (const union diskfs_fhandle *) handle;
+ /* The lookup locks the node and the nput below can drop it. Take the
+ handle before the lock so no start under it waits. */
+ txn = diskfs_journal_start_transaction ();
+
err = diskfs_cached_lookup (f->data.cache_id, &node);
if (err)
{
+ diskfs_journal_stop_transaction (txn);
return err;
}
if (node->dn_stat.st_gen != f->data.gen)
{
diskfs_nput (node);
+ diskfs_journal_stop_transaction (txn);
return ESTALE;
}
@@ -68,6 +75,7 @@ diskfs_S_fsys_getfile (struct diskfs_control *pt,
if (err)
{
diskfs_nput (node);
+ diskfs_journal_stop_transaction (txn);
return err;
}
@@ -93,6 +101,10 @@ diskfs_S_fsys_getfile (struct diskfs_control *pt,
diskfs_nput (node);
+ /* Nothing here changes metadata except a possible drop, and that path
+ asks for sync itself when it needs it. */
+ diskfs_journal_end_transaction (txn, 1, 0);
+
if (! err)
{
*file = ports_get_right (new_cred);
diff --git a/libdiskfs/io-read.c b/libdiskfs/io-read.c
index 78f95d557..11d0b56f5 100644
--- a/libdiskfs/io-read.c
+++ b/libdiskfs/io-read.c
@@ -33,6 +33,7 @@ diskfs_S_io_read (struct protid *cred,
off_t off = offset;
char *buf;
int ourbuf = 0;
+ diskfs_transaction_t *txn;
if (!cred)
return EOPNOTSUPP;
@@ -41,6 +42,9 @@ diskfs_S_io_read (struct protid *cred,
if (!(cred->po->openstat & O_READ))
return EBADF;
+ /* The atime update below writes the inode. Take the handle before the
+ node lock so that start never waits while holding it. */
+ txn = diskfs_journal_start_transaction ();
pthread_mutex_lock (&np->lock);
iohelp_get_conch (&np->conch);
@@ -50,6 +54,7 @@ diskfs_S_io_read (struct protid *cred,
if (off < 0)
{
pthread_mutex_unlock (&np->lock);
+ diskfs_journal_stop_transaction (txn);
return EINVAL;
}
@@ -65,6 +70,7 @@ diskfs_S_io_read (struct protid *cred,
if (buf == MAP_FAILED)
{
pthread_mutex_unlock (&np->lock);
+ diskfs_journal_stop_transaction (txn);
return errno;
}
*data = buf;
@@ -110,5 +116,6 @@ diskfs_S_io_read (struct protid *cred,
munmap (buf, maxread);
pthread_mutex_unlock (&np->lock);
+ diskfs_journal_end_transaction (txn, 1, diskfs_synchronous);
return err;
}
diff --git a/libdiskfs/io-stat.c b/libdiskfs/io-stat.c
index 4d5c63088..005052af4 100644
--- a/libdiskfs/io-stat.c
+++ b/libdiskfs/io-stat.c
@@ -26,11 +26,17 @@ diskfs_S_io_stat (struct protid *cred,
io_statbuf_t *statbuf)
{
struct node *np;
+ diskfs_transaction_t *txn;
if (!cred)
return EOPNOTSUPP;
np = cred->po->np;
+
+ /* diskfs_node_update writes pending times to the inode. Take the
+ handle before the node lock so that start never waits while holding
+ it. */
+ txn = diskfs_journal_start_transaction ();
pthread_mutex_lock (&np->lock);
iohelp_get_conch (&np->conch);
@@ -44,6 +50,7 @@ diskfs_S_io_stat (struct protid *cred,
statbuf->st_mode |= S_IROOT;
pthread_mutex_unlock (&np->lock);
+ diskfs_journal_end_transaction (txn, 1, diskfs_synchronous);
return 0;
}
diff --git a/libdiskfs/node-drop.c b/libdiskfs/node-drop.c
index c1c6cee2f..d11d67f0d 100644
--- a/libdiskfs/node-drop.c
+++ b/libdiskfs/node-drop.c
@@ -38,6 +38,12 @@ void
diskfs_drop_node (struct node *np)
{
mode_t savemode;
+ /* NP is locked. A start may wait for a commit to drain, and must not
+ do so while holding a lock a participant could need, so every path
+ that can reach here from outside an RPC handle (diskfs_release_peropen,
+ diskfs_S_fsys_getfile) opens its handle before locking and this start
+ nests. The remaining outermost caller is the pager thread via
+ diskfs_nrele_light, and a pager thread never waits in start. */
diskfs_transaction_t *txn = diskfs_journal_start_transaction ();
if (np->dn_stat.st_nlink == 0 && !diskfs_readonly)
diff --git a/libdiskfs/peropen-rele.c b/libdiskfs/peropen-rele.c
index 028b5d719..b3881ab8f 100644
--- a/libdiskfs/peropen-rele.c
+++ b/libdiskfs/peropen-rele.c
@@ -21,9 +21,20 @@
void
diskfs_release_peropen (struct peropen *po)
{
+ diskfs_transaction_t *txn;
+
if (refcount_deref (&po->refcnt) > 0)
return;
+ /* This is the last close. Releasing the node below can be its last
+ reference: _diskfs_lastref writes it back under nodecache_lock and
+ diskfs_drop_node truncates and frees it under np->lock. Open the
+ handle before either lock so those starts nest and never wait. This
+ path is reached from the ports clean routine, outside any RPC handle,
+ and from RPCs that already hold one, where this start nests. It
+ never commits: the RPC tail decides that. */
+ txn = diskfs_journal_start_transaction ();
+
if (po->root_parent)
mach_port_deallocate (mach_task_self (), po->root_parent);
@@ -40,4 +51,6 @@ diskfs_release_peropen (struct peropen *po)
free (po->path);
free (po);
+
+ diskfs_journal_stop_transaction (txn);
}
--
2.55.0
