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

[Dave Chinner <david@fromorbit.com>]

On Sat, Mar 06, 2010 at 10:46:10AM -0500, Christoph Hellwig wrote:
> On Sat, Mar 06, 2010 at 12:51:23PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > 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@lst.de>
> 
> 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).

Ah, I killed that union for this patch set - I was using embedded
arrays to cut down on memory allocations for the delayed logging so
there was a:

	union {
		struct xfs_log_iovec	*lv_iovecp;
		struct xfs_log_iovec	lv_iovec[0];
	};

in the structure. I decided that the union wasn't necessary for
these patches (and can be avoided entirely, anyway) but I forgot to
fix up the setup of the structure to remove those comments. I'll
update it as you've suggested.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs