Applied, thanks!
Milos Nikic, le dim. 27 sept. 2026 15:02:59 -0700, a ecrit:
> In all the places libdiskfs interacts with the journal it does, more or
> less, the same thing at the end of the function.
> Extract that logic into a function and call it from those places so we don't
> have to repeat almost identical code many times across RPC handlers.
> ---
> libdiskfs/dir-link.c | 5 +----
> libdiskfs/dir-lookup.c | 5 +----
> libdiskfs/dir-mkdir.c | 5 +----
> libdiskfs/dir-mkfile.c | 5 +----
> libdiskfs/dir-rename.c | 5 +----
> libdiskfs/dir-renamed.c | 5 +----
> libdiskfs/dir-rmdir.c | 5 +----
> libdiskfs/dir-unlink.c | 5 +----
> libdiskfs/file-set-trans.c | 5 +----
> libdiskfs/io-prenotify.c | 5 +----
> libdiskfs/io-sigio.c | 2 +-
> libdiskfs/io-write.c | 5 +----
> libdiskfs/priv.h | 25 +++++++++++++++++++++----
> 13 files changed, 33 insertions(+), 49 deletions(-)
>
> diff --git a/libdiskfs/dir-link.c b/libdiskfs/dir-link.c
> index 608ff2183..7892d49e9 100644
> --- a/libdiskfs/dir-link.c
> +++ b/libdiskfs/dir-link.c
> @@ -147,9 +147,6 @@ diskfs_S_dir_link (struct protid *dircred,
> /* MiG won't do this for us, which it ought to. */
> mach_port_deallocate (mach_task_self (), filecred->pi.port_right);
>
> - if (!err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
> return err;
> }
> diff --git a/libdiskfs/dir-lookup.c b/libdiskfs/dir-lookup.c
> index 77cdbe08c..875036670 100644
> --- a/libdiskfs/dir-lookup.c
> +++ b/libdiskfs/dir-lookup.c
> @@ -573,10 +573,7 @@ diskfs_S_dir_lookup (struct protid *dircred,
> ports_port_deref (newpi);
> if (newpo)
> diskfs_release_peropen (newpo);
> - if (newnode && !err && (diskfs_synchronous || diskfs_journal_needs_sync
> (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, newnode && !err, diskfs_synchronous);
>
> free (relpath);
>
> diff --git a/libdiskfs/dir-mkdir.c b/libdiskfs/dir-mkdir.c
> index 3adaa404e..55295c6be 100644
> --- a/libdiskfs/dir-mkdir.c
> +++ b/libdiskfs/dir-mkdir.c
> @@ -69,9 +69,6 @@ diskfs_S_dir_mkdir (struct protid *dircred,
>
> pthread_mutex_unlock (&dnp->lock);
>
> - if (!error && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !error, diskfs_synchronous);
> return error;
> }
> diff --git a/libdiskfs/dir-mkfile.c b/libdiskfs/dir-mkfile.c
> index 7cffc25cd..ace1320d5 100644
> --- a/libdiskfs/dir-mkfile.c
> +++ b/libdiskfs/dir-mkfile.c
> @@ -92,9 +92,6 @@ diskfs_S_dir_mkfile (struct protid *cred,
> if (np)
> diskfs_nput (np);
>
> - if (!err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
> return err;
> }
> diff --git a/libdiskfs/dir-rename.c b/libdiskfs/dir-rename.c
> index 6328351ad..63aff41f1 100644
> --- a/libdiskfs/dir-rename.c
> +++ b/libdiskfs/dir-rename.c
> @@ -257,10 +257,7 @@ diskfs_S_dir_rename (struct protid *fromcred,
> pthread_mutex_unlock (&fdp->lock);
>
> out:
> - if (! err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
> if (!err)
> mach_port_deallocate (mach_task_self (), tocred->pi.port_right);
> return err;
> diff --git a/libdiskfs/dir-renamed.c b/libdiskfs/dir-renamed.c
> index 67174874d..ed468b539 100644
> --- a/libdiskfs/dir-renamed.c
> +++ b/libdiskfs/dir-renamed.c
> @@ -281,9 +281,6 @@ diskfs_rename_dir (struct node *fdp, struct node *fnp,
> const char *fromname,
> diskfs_drop_dirstat (tdp, ds);
>
> /* FINALIZE TRANSACTION */
> - if (! err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
> return err;
> }
> diff --git a/libdiskfs/dir-rmdir.c b/libdiskfs/dir-rmdir.c
> index 82c20a7ea..0a0f4224c 100644
> --- a/libdiskfs/dir-rmdir.c
> +++ b/libdiskfs/dir-rmdir.c
> @@ -41,10 +41,7 @@ diskfs_S_dir_rmdir (struct protid *dircred,
> if (ds)
> diskfs_drop_dirstat (dnp, ds);
> pthread_mutex_unlock (&dnp->lock);
> - if (!error && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !error, diskfs_synchronous);
>
> return error;
> }
> diff --git a/libdiskfs/dir-unlink.c b/libdiskfs/dir-unlink.c
> index 85df1c977..eaf62f9b8 100644
> --- a/libdiskfs/dir-unlink.c
> +++ b/libdiskfs/dir-unlink.c
> @@ -100,10 +100,7 @@ diskfs_S_dir_unlink (struct protid *dircred,
> mach_port_deallocate (mach_task_self (), control);
> }
>
> - if (diskfs_synchronous || diskfs_journal_needs_sync (txn))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, 1, diskfs_synchronous);
>
> return err;
> }
> diff --git a/libdiskfs/file-set-trans.c b/libdiskfs/file-set-trans.c
> index c35a79591..3aa732866 100644
> --- a/libdiskfs/file-set-trans.c
> +++ b/libdiskfs/file-set-trans.c
> @@ -245,10 +245,7 @@ diskfs_S_file_set_translator (struct protid *cred,
> else
> ret_val = err;
> out:
> - if (! err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction(txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
>
> return ret_val;
> }
> diff --git a/libdiskfs/io-prenotify.c b/libdiskfs/io-prenotify.c
> index c6ff00dd5..b80556d9b 100644
> --- a/libdiskfs/io-prenotify.c
> +++ b/libdiskfs/io-prenotify.c
> @@ -74,9 +74,6 @@ diskfs_S_io_prenotify (struct protid *cred,
> diskfs_notice_filechange (np, FILE_CHANGED_EXTEND, 0, end);
> out:
> pthread_mutex_unlock (&np->lock);
> - if (!err && (diskfs_synchronous || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, diskfs_synchronous);
> return err;
> }
> diff --git a/libdiskfs/io-sigio.c b/libdiskfs/io-sigio.c
> index 0fc226dc1..42a726e46 100644
> --- a/libdiskfs/io-sigio.c
> +++ b/libdiskfs/io-sigio.c
> @@ -39,6 +39,6 @@ diskfs_S_io_sigio (struct protid *cred)
> diskfs_file_update (cred->po->np, 1);
>
> pthread_mutex_unlock (&cred->po->np->lock);
> - diskfs_journal_commit_transaction (txn);
> + diskfs_journal_end_transaction (txn, 1, 1);
> return 0;
> }
> diff --git a/libdiskfs/io-write.c b/libdiskfs/io-write.c
> index 73207f4e3..c07b8c790 100644
> --- a/libdiskfs/io-write.c
> +++ b/libdiskfs/io-write.c
> @@ -96,9 +96,6 @@ diskfs_S_io_write (struct protid *cred,
> diskfs_notice_filechange (np, FILE_CHANGED_WRITE, off, off + nwritten);
> out:
> pthread_mutex_unlock (&np->lock);
> - if (!err && (should_sync || diskfs_journal_needs_sync (txn)))
> - diskfs_journal_commit_transaction (txn);
> - else
> - diskfs_journal_stop_transaction (txn);
> + diskfs_journal_end_transaction (txn, !err, should_sync);
> return err;
> }
> diff --git a/libdiskfs/priv.h b/libdiskfs/priv.h
> index dc3d418fe..071f707ed 100644
> --- a/libdiskfs/priv.h
> +++ b/libdiskfs/priv.h
> @@ -120,6 +120,26 @@ extern pthread_spinlock_t _diskfs_control_lock;
> extern fshelp_fetch_root_callback1_t _diskfs_translator_callback1;
> extern fshelp_fetch_root_callback2_t _diskfs_translator_callback2;
>
> +/* Consume TXN.
> + Wait for the commit when COMMIT_OK is true and either SYNC is true or a
> + participant called diskfs_journal_set_sync. Otherwise stop without
> waiting.
> +
> + COMMIT_OK is false when this RPC failed and must not wait. Paths that
> + return before the operation has done its work, and diskfs_drop_node, call
> + diskfs_journal_stop_transaction directly so they cannot commit. */
> +static inline void
> +diskfs_journal_end_transaction (diskfs_transaction_t *txn,
> + int commit_ok, int sync)
> +{
> + if (!txn)
> + return;
> +
> + if (commit_ok && (sync || diskfs_journal_needs_sync (txn)))
> + diskfs_journal_commit_transaction (txn);
> + else
> + diskfs_journal_stop_transaction (txn);
> +}
> +
> /* This macro locks the node associated with PROTID, and then
> evaluates the expression OPERATION; then it syncs the inode
> (without waiting) and unlocks everything, and then returns
> @@ -143,10 +163,7 @@ extern fshelp_fetch_root_callback2_t
> _diskfs_translator_callback2;
> (OPERATION);
> \
> diskfs_node_update (np, diskfs_synchronous);
> \
> pthread_mutex_unlock (&np->lock); \
> - if (diskfs_synchronous || diskfs_journal_needs_sync (txn)) \
> - diskfs_journal_commit_transaction (txn); \
> - else
> \
> - diskfs_journal_stop_transaction (txn); \
> + diskfs_journal_end_transaction (txn, 1, diskfs_synchronous);
> \
> return err;
> \
> })
>
> --
> 2.55.0
>
--
Samuel
> Allez, soyez sympa ... traduisez-lui "linux"
Linux, c'est comme le miel : c'est vachement bon mais ça attire les
mouches. En plus, ça colle aux doigts et on a du mal à s'en défaire.
-+- TP in: Guide du linuxien pervers - "Barrez vous les mouches !"
