* [PATCH 2/3] kill xfs_ialloc_log_di
@ 2008-10-07 20:21 Christoph Hellwig
2008-10-07 21:21 ` Dave Chinner
2008-10-07 22:24 ` Mark Goodwin
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2008-10-07 20:21 UTC (permalink / raw)
To: xfs
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] kill xfs_ialloc_log_di
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
1 sibling, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2008-10-07 21:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Tue, Oct 07, 2008 at 10:21:57PM +0200, 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.
Looks like a good idea. I'll have to redo some patches on top of
this, but otherwise I think it's fine.
> @@ -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.
> */
That's the patch series I'll need to redo on top of this. ;)
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] kill xfs_ialloc_log_di
2008-10-07 22:24 ` Mark Goodwin
@ 2008-10-07 21:27 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2008-10-07 21:27 UTC (permalink / raw)
To: Mark Goodwin; +Cc: Christoph Hellwig, xfs
On Wed, Oct 08, 2008 at 08:24:49AM +1000, Mark Goodwin wrote:
>
> 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?)
This one is purely cleanup, but I also verified the log replay. The
changes Dave has in the pipe will change the log format.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] kill xfs_ialloc_log_di
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
2008-10-07 21:27 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Mark Goodwin @ 2008-10-07 22:24 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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
-------------------------------------------------------------
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-10-07 21:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2008-10-07 21:27 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox