xfs
[Top] [All Lists]

Re: [PATCH 8/9] xfs: introduce new internal log vector structure

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 8/9] xfs: introduce new internal log vector structure
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Sat, 6 Mar 2010 10:46:10 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1267840284-4652-9-git-send-email-david@xxxxxxxxxxxxx>
References: <1267840284-4652-1-git-send-email-david@xxxxxxxxxxxxx> <1267840284-4652-9-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.19 (2009-01-05)
On Sat, Mar 06, 2010 at 12:51:23PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> The current log IO vector structure is a flat array and not
> extensible. To make it possible to keep separate log IO vectors for
> individual log items, we need a method of chaining log IO vectors
> together.
> 
> Introduce a new log vector type that can be used to wrap the
> existing log IO vectors on use that internally to the log. This
> means that the existing external interface (xfs_log_write) does not
> change and hence no changes to the transaction commit code are
> required.
> 
> This initial use of the new log vectors does not use the chaining
> capability of the new log vector structure - it is not needed to
> implement the flat vector array the current transaction commit path
> creates.

Looks good to me,


Reviewed-by: Christoph Hellwig <hch@xxxxxx>

A few comments below:

> +                     /* the data section must be 32 bit size aligned */
> +                     struct {
> +                         __uint16_t magic;
> +                         __uint16_t pad1;
> +                         __uint32_t pad2; /* may as well make it 64 bits */
> +                     } magic = {
> +                             .magic = XLOG_UNMOUNT_TYPE,
> +                     };
> +                     struct xfs_log_iovec reg = {
> +                             .i_addr = (void *)&magic,
> +                             .i_len = sizeof(magic),
> +                             .i_type = XLOG_REG_TYPE_UNMOUNT,
> +                     };
> +                     struct xfs_log_vec vec = {
> +                             .lv_niovecs = 1,
> +                     };
> +                     /* sigh. c99 initialisers don't work on anon unions */
> +                     vec.lv_iovecp = &reg;

I can't see an anonymous union involved anywhere here, and initializing
these normally works just fine (see patch below).

Index: xfs/fs/xfs/xfs_log.c
===================================================================
--- xfs.orig/fs/xfs/xfs_log.c   2010-03-06 16:40:35.817021839 +0100
+++ xfs/fs/xfs/xfs_log.c        2010-03-06 16:41:49.873004518 +0100
@@ -557,9 +557,8 @@ xfs_log_unmount_write(xfs_mount_t *mp)
                        };
                        struct xfs_log_vec vec = {
                                .lv_niovecs = 1,
+                               .lv_iovecp = &reg,
                        };
-                       /* sigh. c99 initialisers don't work on anon unions */
-                       vec.lv_iovecp = &reg;
 
                        /* remove inited flag */
                        tic->t_flags = 0;
@@ -686,9 +685,8 @@ xfs_log_write(
        int                     error;
        struct xfs_log_vec      vec = {
                .lv_niovecs = nentries,
+               .lv_iovecp = reg,
        };
-       /* sigh. c99 initialisers don't work on anon unions */
-       vec.lv_iovecp = reg;
 
        if (XLOG_FORCED_SHUTDOWN(log))
                return XFS_ERROR(EIO);
@@ -1201,9 +1199,8 @@ xlog_commit_record(
        };
        struct xfs_log_vec vec = {
                .lv_niovecs = 1,
+               .lv_iovecp = &reg,
        };
-       /* sigh. c99 initialisers don't work on anon unions */
-       vec.lv_iovecp = &reg;
 
        ASSERT_ALWAYS(iclog);
        error = xlog_write(log, &vec, ticket, commitlsnp, iclog,

<Prev in Thread] Current Thread [Next in Thread>