xfs
[Top] [All Lists]

Re: [1/3] xfs: remove duplicate buffer flags

To: <hch@xxxxxxxxxxxxx>
Subject: Re: [1/3] xfs: remove duplicate buffer flags
From: "Alex Elder" <aelder@xxxxxxx>
Date: Tue, 19 Jan 2010 17:14:16 -0600
Cc: <xfs@xxxxxxxxxxx>
Thread-index: AcqZXSMO5ZNoVcD/TSaCHMwsoyY3dw==
Thread-topic: Re: [1/3] xfs: remove duplicate buffer flags
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);
>

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [1/3] xfs: remove duplicate buffer flags, Alex Elder <=