I didn't get these in my e-mail, but I've reviewed these three
patches and all look good. Gratuitous commententary below, but
for all three:
Reviewed-by: Alex Elder <aelder@xxxxxxx>
-Alex
[1/3] xfs: remove duplicate buffer flags
I agree that aliases like this obscure the code.
It's possible they were done for reasons of
architectural purity or something, but I think
this change is an improvement.
[2/3] xfs: kill XLOG_VEC_SET_TYPE
Definite improvement.
[3/3] xfs: cleanup up xfs_log_force calling conventions
> Index: xfs/fs/xfs/xfs_log.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_log.c 2010-01-19 10:49:18.100003304 +0100
> +++ xfs/fs/xfs/xfs_log.c 2010-01-19 10:49:33.510003623 +0100
. . .
+void
+xfs_log_force_lsn(
+ xfs_mount_t *mp,
+ xfs_lsn_t lsn,
+ uint flags)
+{
+ int error;
+ error = _xfs_log_force_lsn(mp, lsn, flags, NULL);
+ if (error) {
+ xfs_fs_cmn_err(CE_WARN, mp, "xfs_log_force: "
Function name should be fixed in this statement. I will fix
that for you.
+ "error %d returned.", error);
+ }
+}
. . .
> @@ -3536,13 +3529,14 @@ xfs_log_force_umount(
> }
> spin_unlock(&log->l_grant_lock);
>
> - if (! (log->l_iclog->ic_state & XLOG_STATE_IOERROR)) {
> + if (!(log->l_iclog->ic_state & XLOG_STATE_IOERROR)) {
> ASSERT(!logerror);
> /*
> * Force the incore logs to disk before shutting the
> * log down completely.
> */
> - xlog_state_sync_all(log, XFS_LOG_FORCE|XFS_LOG_SYNC, &dummy);
> + _xfs_log_force(mp, XFS_LOG_SYNC, NULL);
> +
This is a really good change. It was a strange to have this
be the only other spot that called xlog_state_sync_all().
(And of course now you've incorporated the state_sync functions
back into _xfs_log_force*().)
> spin_lock(&log->l_icloglock);
> retval = xlog_state_ioerror(log);
> spin_unlock(&log->l_icloglock);
>
|