public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Mark Goodwin <markgw@sgi.com>
To: Christoph Hellwig <hch@lst.de>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH 2/3] kill xfs_ialloc_log_di
Date: Wed, 08 Oct 2008 08:24:49 +1000	[thread overview]
Message-ID: <48EBE1B1.3090605@sgi.com> (raw)
In-Reply-To: <20081007202157.GB16485@lst.de>


Can we verify dirty log replay immediately after a kernel update works?
(has there been any log fmt revision or is this just pure cleanup?)

Cheers

Christoph Hellwig wrote:
> xfs_ialloc_log_di alwasy logs the full inode core + di_next_unlinked, so
> there's really no need for all the offset magic and we can just call
> xfs_trans_log_buf directly from it's only user.  Also add a comment
> describing what we should do here instead.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6-xfs/fs/xfs/xfs_ialloc.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_ialloc.c	2008-10-07 19:50:47.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_ialloc.c	2008-10-07 19:57:41.000000000 +0200
> @@ -41,68 +41,6 @@
>  #include "xfs_error.h"
>  #include "xfs_bmap.h"
>  
> -/*
> - * Log specified fields for the inode given by bp and off.
> - */
> -STATIC void
> -xfs_ialloc_log_di(
> -	xfs_trans_t	*tp,		/* transaction pointer */
> -	xfs_buf_t	*bp,		/* inode buffer */
> -	int		off,		/* index of inode in buffer */
> -	int		fields)		/* bitmask of fields to log */
> -{
> -	int			first;		/* first byte number */
> -	int			ioffset;	/* off in bytes */
> -	int			last;		/* last byte number */
> -	xfs_mount_t		*mp;		/* mount point structure */
> -	static const short	offsets[] = {	/* field offsets */
> -						/* keep in sync with bits */
> -		offsetof(xfs_dinode_core_t, di_magic),
> -		offsetof(xfs_dinode_core_t, di_mode),
> -		offsetof(xfs_dinode_core_t, di_version),
> -		offsetof(xfs_dinode_core_t, di_format),
> -		offsetof(xfs_dinode_core_t, di_onlink),
> -		offsetof(xfs_dinode_core_t, di_uid),
> -		offsetof(xfs_dinode_core_t, di_gid),
> -		offsetof(xfs_dinode_core_t, di_nlink),
> -		offsetof(xfs_dinode_core_t, di_projid),
> -		offsetof(xfs_dinode_core_t, di_pad),
> -		offsetof(xfs_dinode_core_t, di_atime),
> -		offsetof(xfs_dinode_core_t, di_mtime),
> -		offsetof(xfs_dinode_core_t, di_ctime),
> -		offsetof(xfs_dinode_core_t, di_size),
> -		offsetof(xfs_dinode_core_t, di_nblocks),
> -		offsetof(xfs_dinode_core_t, di_extsize),
> -		offsetof(xfs_dinode_core_t, di_nextents),
> -		offsetof(xfs_dinode_core_t, di_anextents),
> -		offsetof(xfs_dinode_core_t, di_forkoff),
> -		offsetof(xfs_dinode_core_t, di_aformat),
> -		offsetof(xfs_dinode_core_t, di_dmevmask),
> -		offsetof(xfs_dinode_core_t, di_dmstate),
> -		offsetof(xfs_dinode_core_t, di_flags),
> -		offsetof(xfs_dinode_core_t, di_gen),
> -		offsetof(xfs_dinode_t, di_next_unlinked),
> -		offsetof(xfs_dinode_t, di_u),
> -		offsetof(xfs_dinode_t, di_a),
> -		sizeof(xfs_dinode_t)
> -	};
> -
> -
> -	ASSERT(offsetof(xfs_dinode_t, di_core) == 0);
> -	ASSERT((fields & (XFS_DI_U|XFS_DI_A)) == 0);
> -	mp = tp->t_mountp;
> -	/*
> -	 * Get the inode-relative first and last bytes for these fields
> -	 */
> -	xfs_btree_offsets(fields, offsets, XFS_DI_NUM_BITS, &first, &last);
> -	/*
> -	 * Convert to buffer offsets and log it.
> -	 */
> -	ioffset = off << mp->m_sb.sb_inodelog;
> -	first += ioffset;
> -	last += ioffset;
> -	xfs_trans_log_buf(tp, bp, first, last);
> -}
>  
>  /*
>   * Allocation group level functions.
> @@ -406,18 +344,25 @@ xfs_ialloc_ag_alloc(
>  					 XFS_BUF_LOCK);
>  		ASSERT(fbuf);
>  		ASSERT(!XFS_BUF_GETERROR(fbuf));
> +
>  		/*
> -		 * Set initial values for the inodes in this buffer.
> +		 * Initialize all inodes in this buffer and then log them.
> +		 *
> +		 * XXX: It would be much better if we had just one transaction to
> +		 *      log a whole cluster of inodes instead of all the indivdual
> +		 *      transactions causing a lot of log traffic.
>  		 */
>  		xfs_biozero(fbuf, 0, ninodes << args.mp->m_sb.sb_inodelog);
>  		for (i = 0; i < ninodes; i++) {
> +			int	ioffset = i << args.mp->m_sb.sb_inodelog;
> +			uint	isize = sizeof(xfs_dinode_t) + sizeof(__be32);
> +
>  			free = XFS_MAKE_IPTR(args.mp, fbuf, i);
>  			free->di_core.di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
>  			free->di_core.di_version = version;
>  			free->di_core.di_gen = cpu_to_be32(gen);
>  			free->di_next_unlinked = cpu_to_be32(NULLAGINO);
> -			xfs_ialloc_log_di(tp, fbuf, i,
> -				XFS_DI_CORE_BITS | XFS_DI_NEXT_UNLINKED);
> +			xfs_trans_log_buf(tp, fbuf, ioffset, ioffset + isize - 1);
>  		}
>  		xfs_trans_inode_alloc_buf(tp, fbuf);
>  	}
> Index: linux-2.6-xfs/fs/xfs/xfs_dinode.h
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/xfs_dinode.h	2008-10-07 19:51:22.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/xfs_dinode.h	2008-10-07 19:56:16.000000000 +0200
> @@ -102,40 +102,6 @@ typedef struct xfs_dinode
>  #define	XFS_MAXLINK_1		65535U
>  
>  /*
> - * Bit names for logging disk inodes only
> - */
> -#define	XFS_DI_MAGIC		0x0000001
> -#define	XFS_DI_MODE		0x0000002
> -#define	XFS_DI_VERSION		0x0000004
> -#define	XFS_DI_FORMAT		0x0000008
> -#define	XFS_DI_ONLINK		0x0000010
> -#define	XFS_DI_UID		0x0000020
> -#define	XFS_DI_GID		0x0000040
> -#define	XFS_DI_NLINK		0x0000080
> -#define	XFS_DI_PROJID		0x0000100
> -#define	XFS_DI_PAD		0x0000200
> -#define	XFS_DI_ATIME		0x0000400
> -#define	XFS_DI_MTIME		0x0000800
> -#define	XFS_DI_CTIME		0x0001000
> -#define	XFS_DI_SIZE		0x0002000
> -#define	XFS_DI_NBLOCKS		0x0004000
> -#define	XFS_DI_EXTSIZE		0x0008000
> -#define	XFS_DI_NEXTENTS		0x0010000
> -#define	XFS_DI_NAEXTENTS	0x0020000
> -#define	XFS_DI_FORKOFF		0x0040000
> -#define	XFS_DI_AFORMAT		0x0080000
> -#define	XFS_DI_DMEVMASK		0x0100000
> -#define	XFS_DI_DMSTATE		0x0200000
> -#define	XFS_DI_FLAGS		0x0400000
> -#define	XFS_DI_GEN		0x0800000
> -#define	XFS_DI_NEXT_UNLINKED	0x1000000
> -#define	XFS_DI_U		0x2000000
> -#define	XFS_DI_A		0x4000000
> -#define	XFS_DI_NUM_BITS		27
> -#define	XFS_DI_ALL_BITS		((1 << XFS_DI_NUM_BITS) - 1)
> -#define	XFS_DI_CORE_BITS	(XFS_DI_ALL_BITS & ~(XFS_DI_U|XFS_DI_A))
> -
> -/*
>   * Values for di_format
>   */
>  typedef enum xfs_dinode_fmt
> 
> 

-- 

  Mark Goodwin                                  markgw@sgi.com
  Engineering Manager for XFS and PCP    Phone: +61-3-99631937
  SGI Australian Software Group           Cell: +61-4-18969583
-------------------------------------------------------------

  parent reply	other threads:[~2008-10-07 21:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-07 20:21 [PATCH 2/3] kill xfs_ialloc_log_di Christoph Hellwig
2008-10-07 21:21 ` Dave Chinner
2008-10-07 22:24 ` Mark Goodwin [this message]
2008-10-07 21:27   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48EBE1B1.3090605@sgi.com \
    --to=markgw@sgi.com \
    --cc=hch@lst.de \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox