Re: [PATCH 15/71] xfs: create refcount update intent log items

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 06, 2016 at 08:16:03AM -0700, Christoph Hellwig wrote:
> > +/*
> > + * This is the structure used to lay out a cui log item in the
> > + * log.  The cui_extents field is a variable size array whose
> > + * size is given by cui_nextents.
> > + */
> > +struct xfs_cui_log_format {
> > +	__uint16_t		cui_type;	/* cui log item type */
> > +	__uint16_t		cui_size;	/* size of this item */
> > +	__uint32_t		cui_nextents;	/* # extents to free */
> > +	__uint64_t		cui_id;		/* cui identifier */
> > +	struct xfs_phys_extent	cui_extents[1];	/* array of extents */
> 
> Please define this as a proper variable length extent, e.g.
> 
> 	struct xfs_phys_extent	cui_extents[];
> 
> and get rid of the one-off arithmentics in xfs_cui_copy_format and
> xfs_cui_init.

Ok.

> > +int
> > +xfs_cui_copy_format(
> > +	struct xfs_log_iovec		*buf,
> > +	struct xfs_cui_log_format	*dst_cui_fmt)
> > +{
> > +	struct xfs_cui_log_format	*src_cui_fmt;
> > +	uint				len;
> > +
> > +	src_cui_fmt = buf->i_addr;
> > +	len = sizeof(struct xfs_cui_log_format) +
> > +			(src_cui_fmt->cui_nextents - 1) *
> > +			sizeof(struct xfs_phys_extent);
> > +
> > +	if (buf->i_len == len) {
> > +		memcpy((char *)dst_cui_fmt, (char *)src_cui_fmt, len);
> > +		return 0;
> > +	}
> > +	return -EFSCORRUPTED;
> 
> 
> Wouldn't life be a simpler if we simply opencoded this in
> xlog_recover_cui_pass2?  Also no need for the casts in the memcpy
> arguments.

<shrug> I'm relatively indifferent either way, though I noticed that
EFIs have a separate copy_format function in xfs_extfree_item.c which
reduces the amount of clutter in xfs_log_recover.c.  It seemed more
neat to keep all the log item functions corralled within a separate
file rather than putting them in xfs_log_recover.c, so I maintained
that pattern for the rui/cui/bui items.

In a semi-related side note I've been toying with turning on LTO in
xfsprogs to see how much it shrinks all the XFS code.  Code size goes
down considerably (repair shrinks by nearly 1MB!) but wow the
resulting tangle can be a PITA to debug.  :(

--D
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux