* [PATCH 0/7] inode allocation cleanups
@ 2009-05-12 21:16 Christoph Hellwig
2009-05-12 21:16 ` [PATCH 1/7] xfs: factor out inode initialisation Christoph Hellwig
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
This series contains various cleanups and refactorings to xfs_ialloc.c,
some of them are my own and some are from Dave Chinner.
The last patch is also from Dave and speeds up the search for free inodes
in allocation groups with lots of inodes. I ported it ontop of all
the previous patches, which made it a lot cleaner.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/7] xfs: factor out inode initialisation
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 2/7] xfs: improve xfs_inobt_get_rec prototype Christoph Hellwig
` (6 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-icreate-factor-inode-stamping --]
[-- Type: text/plain, Size: 9058 bytes --]
From: Dave Chinner <david@fromorbit.com>
Factor out code to initialize new inode clusters into a function of it's own.
This keeps xfs_ialloc_ag_alloc smaller and better structured and enables a
future inode cluster initialization transaction. Also initialize the agno
variable earlier in xfs_ialloc_ag_alloc to avoid repeated byte swaps.
[hch: The original patch is from Dave from his unpublished inode create
transaction patch series, with some modifcation by to apply stand-alone]
Signed-off-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-05-12 17:11:12.107658861 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2009-05-12 17:14:55.411784530 +0200
@@ -153,6 +153,87 @@ xfs_inobt_get_rec(
}
/*
+ * Initialise a new set of inodes.
+ */
+STATIC void
+xfs_ialloc_inode_init(
+ struct xfs_mount *mp,
+ struct xfs_trans *tp,
+ xfs_agnumber_t agno,
+ xfs_agblock_t agbno,
+ xfs_agblock_t length,
+ unsigned int gen)
+{
+ struct xfs_buf *fbuf;
+ struct xfs_dinode *free;
+ int blks_per_cluster, nbufs, ninodes;
+ int version;
+ int i, j;
+ xfs_daddr_t d;
+
+ /*
+ * Loop over the new block(s), filling in the inodes.
+ * For small block sizes, manipulate the inodes in buffers
+ * which are multiples of the blocks size.
+ */
+ if (mp->m_sb.sb_blocksize >= XFS_INODE_CLUSTER_SIZE(mp)) {
+ blks_per_cluster = 1;
+ nbufs = length;
+ ninodes = mp->m_sb.sb_inopblock;
+ } else {
+ blks_per_cluster = XFS_INODE_CLUSTER_SIZE(mp) /
+ mp->m_sb.sb_blocksize;
+ nbufs = length / blks_per_cluster;
+ ninodes = blks_per_cluster * mp->m_sb.sb_inopblock;
+ }
+
+ /*
+ * Figure out what version number to use in the inodes we create.
+ * If the superblock version has caught up to the one that supports
+ * the new inode format, then use the new inode version. Otherwise
+ * use the old version so that old kernels will continue to be
+ * able to use the file system.
+ */
+ if (xfs_sb_version_hasnlink(&mp->m_sb))
+ version = 2;
+ else
+ version = 1;
+
+ for (j = 0; j < nbufs; j++) {
+ /*
+ * Get the block.
+ */
+ d = XFS_AGB_TO_DADDR(mp, agno, agbno + (j * blks_per_cluster));
+ fbuf = xfs_trans_get_buf(tp, mp->m_ddev_targp, d,
+ mp->m_bsize * blks_per_cluster,
+ XFS_BUF_LOCK);
+ ASSERT(fbuf);
+ ASSERT(!XFS_BUF_GETERROR(fbuf));
+
+ /*
+ * 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
+ * individual transactions causing a lot of log traffic.
+ */
+ xfs_biozero(fbuf, 0, ninodes << mp->m_sb.sb_inodelog);
+ for (i = 0; i < ninodes; i++) {
+ int ioffset = i << mp->m_sb.sb_inodelog;
+ uint isize = sizeof(struct xfs_dinode);
+
+ free = xfs_make_iptr(mp, fbuf, i);
+ free->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
+ free->di_version = version;
+ free->di_gen = cpu_to_be32(gen);
+ free->di_next_unlinked = cpu_to_be32(NULLAGINO);
+ xfs_trans_log_buf(tp, fbuf, ioffset, ioffset + isize - 1);
+ }
+ xfs_trans_inode_alloc_buf(tp, fbuf);
+ }
+}
+
+/*
* Allocate new inodes in the allocation group specified by agbp.
* Return 0 for success, else error code.
*/
@@ -164,24 +245,15 @@ xfs_ialloc_ag_alloc(
{
xfs_agi_t *agi; /* allocation group header */
xfs_alloc_arg_t args; /* allocation argument structure */
- int blks_per_cluster; /* fs blocks per inode cluster */
xfs_btree_cur_t *cur; /* inode btree cursor */
- xfs_daddr_t d; /* disk addr of buffer */
xfs_agnumber_t agno;
int error;
- xfs_buf_t *fbuf; /* new free inodes' buffer */
- xfs_dinode_t *free; /* new free inode structure */
- int i; /* inode counter */
- int j; /* block counter */
- int nbufs; /* num bufs of new inodes */
+ int i;
xfs_agino_t newino; /* new first inode's number */
xfs_agino_t newlen; /* new number of inodes */
- int ninodes; /* num inodes per buf */
xfs_agino_t thisino; /* current inode number, for loop */
- int version; /* inode version number to use */
int isaligned = 0; /* inode allocation at stripe unit */
/* boundary */
- unsigned int gen;
args.tp = tp;
args.mp = tp->t_mountp;
@@ -202,12 +274,12 @@ xfs_ialloc_ag_alloc(
*/
agi = XFS_BUF_TO_AGI(agbp);
newino = be32_to_cpu(agi->agi_newino);
+ agno = be32_to_cpu(agi->agi_seqno);
args.agbno = XFS_AGINO_TO_AGBNO(args.mp, newino) +
XFS_IALLOC_BLOCKS(args.mp);
if (likely(newino != NULLAGINO &&
(args.agbno < be32_to_cpu(agi->agi_length)))) {
- args.fsbno = XFS_AGB_TO_FSB(args.mp,
- be32_to_cpu(agi->agi_seqno), args.agbno);
+ args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
args.type = XFS_ALLOCTYPE_THIS_BNO;
args.mod = args.total = args.wasdel = args.isfl =
args.userdata = args.minalignslop = 0;
@@ -258,8 +330,7 @@ xfs_ialloc_ag_alloc(
* For now, just allocate blocks up front.
*/
args.agbno = be32_to_cpu(agi->agi_root);
- args.fsbno = XFS_AGB_TO_FSB(args.mp,
- be32_to_cpu(agi->agi_seqno), args.agbno);
+ args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
/*
* Allocate a fixed-size extent of inodes.
*/
@@ -282,8 +353,7 @@ xfs_ialloc_ag_alloc(
if (isaligned && args.fsbno == NULLFSBLOCK) {
args.type = XFS_ALLOCTYPE_NEAR_BNO;
args.agbno = be32_to_cpu(agi->agi_root);
- args.fsbno = XFS_AGB_TO_FSB(args.mp,
- be32_to_cpu(agi->agi_seqno), args.agbno);
+ args.fsbno = XFS_AGB_TO_FSB(args.mp, agno, args.agbno);
args.alignment = xfs_ialloc_cluster_alignment(&args);
if ((error = xfs_alloc_vextent(&args)))
return error;
@@ -294,85 +364,30 @@ xfs_ialloc_ag_alloc(
return 0;
}
ASSERT(args.len == args.minlen);
- /*
- * Convert the results.
- */
- newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
- /*
- * Loop over the new block(s), filling in the inodes.
- * For small block sizes, manipulate the inodes in buffers
- * which are multiples of the blocks size.
- */
- if (args.mp->m_sb.sb_blocksize >= XFS_INODE_CLUSTER_SIZE(args.mp)) {
- blks_per_cluster = 1;
- nbufs = (int)args.len;
- ninodes = args.mp->m_sb.sb_inopblock;
- } else {
- blks_per_cluster = XFS_INODE_CLUSTER_SIZE(args.mp) /
- args.mp->m_sb.sb_blocksize;
- nbufs = (int)args.len / blks_per_cluster;
- ninodes = blks_per_cluster * args.mp->m_sb.sb_inopblock;
- }
- /*
- * Figure out what version number to use in the inodes we create.
- * If the superblock version has caught up to the one that supports
- * the new inode format, then use the new inode version. Otherwise
- * use the old version so that old kernels will continue to be
- * able to use the file system.
- */
- if (xfs_sb_version_hasnlink(&args.mp->m_sb))
- version = 2;
- else
- version = 1;
/*
+ * Stamp and write the inode buffers.
+ *
* Seed the new inode cluster with a random generation number. This
* prevents short-term reuse of generation numbers if a chunk is
* freed and then immediately reallocated. We use random numbers
* rather than a linear progression to prevent the next generation
* number from being easily guessable.
*/
- gen = random32();
- for (j = 0; j < nbufs; j++) {
- /*
- * Get the block.
- */
- d = XFS_AGB_TO_DADDR(args.mp, be32_to_cpu(agi->agi_seqno),
- args.agbno + (j * blks_per_cluster));
- fbuf = xfs_trans_get_buf(tp, args.mp->m_ddev_targp, d,
- args.mp->m_bsize * blks_per_cluster,
- XFS_BUF_LOCK);
- ASSERT(fbuf);
- ASSERT(!XFS_BUF_GETERROR(fbuf));
+ xfs_ialloc_inode_init(args.mp, tp, agno, args.agbno, args.len,
+ random32());
- /*
- * 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 individual
- * 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(struct xfs_dinode);
-
- free = xfs_make_iptr(args.mp, fbuf, i);
- free->di_magic = cpu_to_be16(XFS_DINODE_MAGIC);
- free->di_version = version;
- free->di_gen = cpu_to_be32(gen);
- free->di_next_unlinked = cpu_to_be32(NULLAGINO);
- xfs_trans_log_buf(tp, fbuf, ioffset, ioffset + isize - 1);
- }
- xfs_trans_inode_alloc_buf(tp, fbuf);
- }
+ /*
+ * Convert the results.
+ */
+ newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
be32_add_cpu(&agi->agi_count, newlen);
be32_add_cpu(&agi->agi_freecount, newlen);
- agno = be32_to_cpu(agi->agi_seqno);
down_read(&args.mp->m_peraglock);
args.mp->m_perag[agno].pagi_freecount += newlen;
up_read(&args.mp->m_peraglock);
agi->agi_newino = cpu_to_be32(newino);
+
/*
* Insert records describing the new inode chunk into the btree.
*/
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/7] xfs: improve xfs_inobt_get_rec prototype
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
2009-05-12 21:16 ` [PATCH 1/7] xfs: factor out inode initialisation Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 3/7] xfs: improve xfs_inobt_update prototype Christoph Hellwig
` (5 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-better-xfs_inobt_get_rec-prototype --]
[-- Type: text/plain, Size: 13341 bytes --]
Most callers of xfs_inobt_get_rec need to fill a xfs_inobt_rec_incore_t, and
those who don't yet are fine with a xfs_inobt_rec_incore_t, instead of the
three individual variables, too. So just change xfs_inobt_get_rec to write
the output into a xfs_inobt_rec_incore_t directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-05-12 22:42:28.693659122 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2009-05-12 22:45:59.518658886 +0200
@@ -135,9 +135,7 @@ xfs_inobt_update(
int /* error */
xfs_inobt_get_rec(
struct xfs_btree_cur *cur, /* btree cursor */
- xfs_agino_t *ino, /* output: starting inode of chunk */
- __int32_t *fcnt, /* output: number of free inodes */
- xfs_inofree_t *free, /* output: free inode mask */
+ xfs_inobt_rec_incore_t *irec, /* btree record */
int *stat) /* output: success/failure */
{
union xfs_btree_rec *rec;
@@ -145,9 +143,9 @@ xfs_inobt_get_rec(
error = xfs_btree_get_rec(cur, &rec, stat);
if (!error && *stat == 1) {
- *ino = be32_to_cpu(rec->inobt.ir_startino);
- *fcnt = be32_to_cpu(rec->inobt.ir_freecount);
- *free = be64_to_cpu(rec->inobt.ir_free);
+ irec->ir_startino = be32_to_cpu(rec->inobt.ir_startino);
+ irec->ir_freecount = be32_to_cpu(rec->inobt.ir_freecount);
+ irec->ir_free = be64_to_cpu(rec->inobt.ir_free);
}
return error;
}
@@ -746,8 +744,8 @@ nextag:
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
do {
- if ((error = xfs_inobt_get_rec(cur, &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
freecount += rec.ir_freecount;
@@ -766,8 +764,7 @@ nextag:
if ((error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i)))
goto error0;
if (i != 0 &&
- (error = xfs_inobt_get_rec(cur, &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free, &j)) == 0 &&
+ (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
j == 1 &&
rec.ir_freecount > 0) {
/*
@@ -799,10 +796,8 @@ nextag:
goto error1;
doneleft = !i;
if (!doneleft) {
- if ((error = xfs_inobt_get_rec(tcur,
- &trec.ir_startino,
- &trec.ir_freecount,
- &trec.ir_free, &i)))
+ error = xfs_inobt_get_rec(tcur, &trec, &i);
+ if (error)
goto error1;
XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
}
@@ -813,10 +808,8 @@ nextag:
goto error1;
doneright = !i;
if (!doneright) {
- if ((error = xfs_inobt_get_rec(cur,
- &rec.ir_startino,
- &rec.ir_freecount,
- &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error1;
XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
}
@@ -876,11 +869,9 @@ nextag:
goto error1;
doneleft = !i;
if (!doneleft) {
- if ((error = xfs_inobt_get_rec(
- tcur,
- &trec.ir_startino,
- &trec.ir_freecount,
- &trec.ir_free, &i)))
+ error = xfs_inobt_get_rec(
+ tcur, &trec, &i);
+ if (error)
goto error1;
XFS_WANT_CORRUPTED_GOTO(i == 1,
error1);
@@ -896,11 +887,9 @@ nextag:
goto error1;
doneright = !i;
if (!doneright) {
- if ((error = xfs_inobt_get_rec(
- cur,
- &rec.ir_startino,
- &rec.ir_freecount,
- &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(
+ cur, &rec, &i);
+ if (error)
goto error1;
XFS_WANT_CORRUPTED_GOTO(i == 1,
error1);
@@ -919,8 +908,7 @@ nextag:
be32_to_cpu(agi->agi_newino), 0, 0, &i)))
goto error0;
if (i == 1 &&
- (error = xfs_inobt_get_rec(cur, &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free, &j)) == 0 &&
+ (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
j == 1 &&
rec.ir_freecount > 0) {
/*
@@ -938,10 +926,8 @@ nextag:
goto error0;
ASSERT(i == 1);
for (;;) {
- if ((error = xfs_inobt_get_rec(cur,
- &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free,
- &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
if (rec.ir_freecount > 0)
@@ -975,8 +961,8 @@ nextag:
if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
goto error0;
do {
- if ((error = xfs_inobt_get_rec(cur, &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
freecount += rec.ir_freecount;
@@ -1084,8 +1070,8 @@ xfs_difree(
if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
goto error0;
do {
- if ((error = xfs_inobt_get_rec(cur, &rec.ir_startino,
- &rec.ir_freecount, &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error0;
if (i) {
freecount += rec.ir_freecount;
@@ -1107,8 +1093,8 @@ xfs_difree(
goto error0;
}
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- if ((error = xfs_inobt_get_rec(cur, &rec.ir_startino, &rec.ir_freecount,
- &rec.ir_free, &i))) {
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error) {
cmn_err(CE_WARN,
"xfs_difree: xfs_inobt_get_rec() returned an error %d on %s. Returning error.",
error, mp->m_fsname);
@@ -1187,10 +1173,8 @@ xfs_difree(
if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
goto error0;
do {
- if ((error = xfs_inobt_get_rec(cur,
- &rec.ir_startino,
- &rec.ir_freecount,
- &rec.ir_free, &i)))
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
goto error0;
if (i) {
freecount += rec.ir_freecount;
@@ -1312,9 +1296,7 @@ xfs_imap(
chunk_agbno = agbno - offset_agbno;
} else {
xfs_btree_cur_t *cur; /* inode btree cursor */
- xfs_agino_t chunk_agino; /* first agino in inode chunk */
- __int32_t chunk_cnt; /* count of free inodes in chunk */
- xfs_inofree_t chunk_free; /* mask of free inodes in chunk */
+ xfs_inobt_rec_incore_t chunk_rec;
xfs_buf_t *agbp; /* agi buffer */
int i; /* temp state */
@@ -1337,8 +1319,7 @@ xfs_imap(
goto error0;
}
- error = xfs_inobt_get_rec(cur, &chunk_agino, &chunk_cnt,
- &chunk_free, &i);
+ error = xfs_inobt_get_rec(cur, &chunk_rec, &i);
if (error) {
xfs_fs_cmn_err(CE_ALERT, mp, "xfs_imap: "
"xfs_inobt_get_rec() failed");
@@ -1356,7 +1337,7 @@ xfs_imap(
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
if (error)
return error;
- chunk_agbno = XFS_AGINO_TO_AGBNO(mp, chunk_agino);
+ chunk_agbno = XFS_AGINO_TO_AGBNO(mp, chunk_rec.ir_startino);
offset_agbno = agbno - chunk_agbno;
}
Index: xfs/fs/xfs/xfs_ialloc.h
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.h 2009-05-12 22:42:25.220812390 +0200
+++ xfs/fs/xfs/xfs_ialloc.h 2009-05-12 22:45:59.518658886 +0200
@@ -166,7 +166,7 @@ int xfs_inobt_lookup_le(struct xfs_btree
/*
* Get the data from the pointed-to record.
*/
-extern int xfs_inobt_get_rec(struct xfs_btree_cur *cur, xfs_agino_t *ino,
- __int32_t *fcnt, xfs_inofree_t *free, int *stat);
+extern int xfs_inobt_get_rec(struct xfs_btree_cur *cur,
+ xfs_inobt_rec_incore_t *rec, int *stat);
#endif /* __XFS_IALLOC_H__ */
Index: xfs/fs/xfs/xfs_itable.c
===================================================================
--- xfs.orig/fs/xfs/xfs_itable.c 2009-05-12 22:42:25.233785061 +0200
+++ xfs/fs/xfs/xfs_itable.c 2009-05-12 22:45:59.519658874 +0200
@@ -353,9 +353,6 @@ xfs_bulkstat(
int end_of_ag; /* set if we've seen the ag end */
int error; /* error code */
int fmterror;/* bulkstat formatter result */
- __int32_t gcnt; /* current btree rec's count */
- xfs_inofree_t gfree; /* current btree rec's free mask */
- xfs_agino_t gino; /* current btree rec's start inode */
int i; /* loop index */
int icount; /* count of inodes good in irbuf */
size_t irbsize; /* size of irec buffer in bytes */
@@ -442,6 +439,8 @@ xfs_bulkstat(
* we need to get the remainder of the chunk we're in.
*/
if (agino > 0) {
+ xfs_inobt_rec_incore_t r;
+
/*
* Lookup the inode chunk that this inode lives in.
*/
@@ -449,33 +448,33 @@ xfs_bulkstat(
if (!error && /* no I/O error */
tmp && /* lookup succeeded */
/* got the record, should always work */
- !(error = xfs_inobt_get_rec(cur, &gino, &gcnt,
- &gfree, &i)) &&
+ !(error = xfs_inobt_get_rec(cur, &r, &i)) &&
i == 1 &&
/* this is the right chunk */
- agino < gino + XFS_INODES_PER_CHUNK &&
+ agino < r.ir_startino + XFS_INODES_PER_CHUNK &&
/* lastino was not last in chunk */
- (chunkidx = agino - gino + 1) <
+ (chunkidx = agino - r.ir_startino + 1) <
XFS_INODES_PER_CHUNK &&
/* there are some left allocated */
xfs_inobt_maskn(chunkidx,
- XFS_INODES_PER_CHUNK - chunkidx) & ~gfree) {
+ XFS_INODES_PER_CHUNK - chunkidx) &
+ ~r.ir_free) {
/*
* Grab the chunk record. Mark all the
* uninteresting inodes (because they're
* before our start point) free.
*/
for (i = 0; i < chunkidx; i++) {
- if (XFS_INOBT_MASK(i) & ~gfree)
- gcnt++;
+ if (XFS_INOBT_MASK(i) & ~r.ir_free)
+ r.ir_freecount++;
}
- gfree |= xfs_inobt_maskn(0, chunkidx);
- irbp->ir_startino = gino;
- irbp->ir_freecount = gcnt;
- irbp->ir_free = gfree;
+ r.ir_free |= xfs_inobt_maskn(0, chunkidx);
+ irbp->ir_startino = r.ir_startino;
+ irbp->ir_freecount = r.ir_freecount;
+ irbp->ir_free = r.ir_free;
irbp++;
- agino = gino + XFS_INODES_PER_CHUNK;
- icount = XFS_INODES_PER_CHUNK - gcnt;
+ agino = r.ir_startino + XFS_INODES_PER_CHUNK;
+ icount = XFS_INODES_PER_CHUNK - r.ir_freecount;
} else {
/*
* If any of those tests failed, bump the
@@ -501,6 +500,8 @@ xfs_bulkstat(
* until we run out of inodes or space in the buffer.
*/
while (irbp < irbufend && icount < ubcount) {
+ xfs_inobt_rec_incore_t r;
+
/*
* Loop as long as we're unable to read the
* inode btree.
@@ -518,43 +519,47 @@ xfs_bulkstat(
* If ran off the end of the ag either with an error,
* or the normal way, set end and stop collecting.
*/
- if (error ||
- (error = xfs_inobt_get_rec(cur, &gino, &gcnt,
- &gfree, &i)) ||
- i == 0) {
+ if (error) {
+ end_of_ag = 1;
+ break;
+ }
+
+ error = xfs_inobt_get_rec(cur, &r, &i);
+ if (error || i == 0) {
end_of_ag = 1;
break;
}
+
/*
* If this chunk has any allocated inodes, save it.
* Also start read-ahead now for this chunk.
*/
- if (gcnt < XFS_INODES_PER_CHUNK) {
+ if (r.ir_freecount < XFS_INODES_PER_CHUNK) {
/*
* Loop over all clusters in the next chunk.
* Do a readahead if there are any allocated
* inodes in that cluster.
*/
- for (agbno = XFS_AGINO_TO_AGBNO(mp, gino),
- chunkidx = 0;
+ agbno = XFS_AGINO_TO_AGBNO(mp, r.ir_startino);
+ for (chunkidx = 0;
chunkidx < XFS_INODES_PER_CHUNK;
chunkidx += nicluster,
agbno += nbcluster) {
- if (xfs_inobt_maskn(chunkidx,
- nicluster) & ~gfree)
+ if (xfs_inobt_maskn(chunkidx, nicluster)
+ & ~r.ir_free)
xfs_btree_reada_bufs(mp, agno,
agbno, nbcluster);
}
- irbp->ir_startino = gino;
- irbp->ir_freecount = gcnt;
- irbp->ir_free = gfree;
+ irbp->ir_startino = r.ir_startino;
+ irbp->ir_freecount = r.ir_freecount;
+ irbp->ir_free = r.ir_free;
irbp++;
- icount += XFS_INODES_PER_CHUNK - gcnt;
+ icount += XFS_INODES_PER_CHUNK - r.ir_freecount;
}
/*
* Set agino to after this chunk and bump the cursor.
*/
- agino = gino + XFS_INODES_PER_CHUNK;
+ agino = r.ir_startino + XFS_INODES_PER_CHUNK;
error = xfs_btree_increment(cur, 0, &tmp);
cond_resched();
}
@@ -820,9 +825,7 @@ xfs_inumbers(
int bufidx;
xfs_btree_cur_t *cur;
int error;
- __int32_t gcnt;
- xfs_inofree_t gfree;
- xfs_agino_t gino;
+ xfs_inobt_rec_incore_t r;
int i;
xfs_ino_t ino;
int left;
@@ -870,9 +873,8 @@ xfs_inumbers(
continue;
}
}
- if ((error = xfs_inobt_get_rec(cur, &gino, &gcnt, &gfree,
- &i)) ||
- i == 0) {
+ error = xfs_inobt_get_rec(cur, &r, &i);
+ if (error || i == 0) {
xfs_buf_relse(agbp);
agbp = NULL;
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
@@ -881,10 +883,12 @@ xfs_inumbers(
agino = 0;
continue;
}
- agino = gino + XFS_INODES_PER_CHUNK - 1;
- buffer[bufidx].xi_startino = XFS_AGINO_TO_INO(mp, agno, gino);
- buffer[bufidx].xi_alloccount = XFS_INODES_PER_CHUNK - gcnt;
- buffer[bufidx].xi_allocmask = ~gfree;
+ agino = r.ir_startino + XFS_INODES_PER_CHUNK - 1;
+ buffer[bufidx].xi_startino =
+ XFS_AGINO_TO_INO(mp, agno, r.ir_startino);
+ buffer[bufidx].xi_alloccount =
+ XFS_INODES_PER_CHUNK - r.ir_freecount;
+ buffer[bufidx].xi_allocmask = ~r.ir_free;
bufidx++;
left--;
if (bufidx == bcount) {
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/7] xfs: improve xfs_inobt_update prototype
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
2009-05-12 21:16 ` [PATCH 1/7] xfs: factor out inode initialisation Christoph Hellwig
2009-05-12 21:16 ` [PATCH 2/7] xfs: improve xfs_inobt_get_rec prototype Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 4/7] xfs: factor out debug checks from xfs_dialloc and xfs_difree Christoph Hellwig
` (4 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-better-xfs_inobt_update-prototype --]
[-- Type: text/plain, Size: 2391 bytes --]
Both callers of xfs_inobt_update have the record in form of a
xfs_inobt_rec_incore_t, so just pass a pointer to it instead of the
individual variables.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-02-25 15:39:43.062918771 +0100
+++ xfs/fs/xfs/xfs_ialloc.c 2009-02-25 15:42:10.898919328 +0100
@@ -110,22 +110,19 @@ xfs_inobt_lookup_le(
}
/*
- * Update the record referred to by cur to the value given
- * by [ino, fcnt, free].
+ * Update the record referred to by cur to the value given.
* This either works (return 0) or gets an EFSCORRUPTED error.
*/
STATIC int /* error */
xfs_inobt_update(
struct xfs_btree_cur *cur, /* btree cursor */
- xfs_agino_t ino, /* starting inode of chunk */
- __int32_t fcnt, /* free inode count */
- xfs_inofree_t free) /* free inode mask */
+ xfs_inobt_rec_incore_t *irec) /* btree record */
{
union xfs_btree_rec rec;
- rec.inobt.ir_startino = cpu_to_be32(ino);
- rec.inobt.ir_freecount = cpu_to_be32(fcnt);
- rec.inobt.ir_free = cpu_to_be64(free);
+ rec.inobt.ir_startino = cpu_to_be32(irec->ir_startino);
+ rec.inobt.ir_freecount = cpu_to_be32(irec->ir_freecount);
+ rec.inobt.ir_free = cpu_to_be64(irec->ir_free);
return xfs_btree_update(cur, &rec);
}
@@ -946,8 +943,8 @@ nextag:
ino = XFS_AGINO_TO_INO(mp, agno, rec.ir_startino + offset);
rec.ir_free &= ~XFS_INOBT_MASK(offset);
rec.ir_freecount--;
- if ((error = xfs_inobt_update(cur, rec.ir_startino, rec.ir_freecount,
- rec.ir_free)))
+ error = xfs_inobt_update(cur, &rec);
+ if (error)
goto error0;
be32_add_cpu(&agi->agi_freecount, -1);
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
@@ -1149,12 +1146,14 @@ xfs_difree(
} else {
*delete = 0;
- if ((error = xfs_inobt_update(cur, rec.ir_startino, rec.ir_freecount, rec.ir_free))) {
+ error = xfs_inobt_update(cur, &rec);
+ if (error) {
cmn_err(CE_WARN,
- "xfs_difree: xfs_inobt_update() returned an error %d on %s. Returning error.",
+ "xfs_difree: xfs_inobt_update returned an error %d on %s.",
error, mp->m_fsname);
goto error0;
}
+
/*
* Change the inode free counts and log the ag/sb changes.
*/
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/7] xfs: factor out debug checks from xfs_dialloc and xfs_difree
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
` (2 preceding siblings ...)
2009-05-12 21:16 ` [PATCH 3/7] xfs: improve xfs_inobt_update prototype Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
` (3 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-factor-freecount-checks --]
[-- Type: text/plain, Size: 4644 bytes --]
From: Dave Chinner <dgc@sgi.com>
Factor out a common helper from repeated debug checks in xfs_dialloc and
xfs_difree.
[hch: split out from Dave's dynamic allocation policy patches]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-02-25 15:42:10.898919328 +0100
+++ xfs/fs/xfs/xfs_ialloc.c 2009-02-25 15:44:27.680921419 +0100
@@ -148,6 +148,47 @@ xfs_inobt_get_rec(
}
/*
+ * Verify that the number of free inodes in the AGI is correct.
+ */
+#ifdef DEBUG
+STATIC int
+xfs_check_agi_freecount(
+ struct xfs_btree_cur *cur,
+ struct xfs_agi *agi)
+{
+ if (cur->bc_nlevels == 1) {
+ xfs_inobt_rec_incore_t rec;
+ int freecount = 0;
+ int error;
+ int i;
+
+ error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+ if (error)
+ return error;
+
+ do {
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
+ return error;
+
+ if (i) {
+ freecount += rec.ir_freecount;
+ error = xfs_btree_increment(cur, 0, &i);
+ if (error)
+ return error;
+ }
+ } while (i == 1);
+
+ if (!XFS_FORCED_SHUTDOWN(cur->bc_mp))
+ ASSERT(freecount == be32_to_cpu(agi->agi_freecount));
+ }
+ return 0;
+}
+#else
+#define xfs_check_agi_freecount(cur, agi) 0
+#endif
+
+/*
* Initialise a new set of inodes.
*/
STATIC void
@@ -548,6 +589,7 @@ nextag:
}
}
+
/*
* Visible inode allocation functions.
*/
@@ -733,27 +775,11 @@ nextag:
*/
if (!pagino)
pagino = be32_to_cpu(agi->agi_newino);
-#ifdef DEBUG
- if (cur->bc_nlevels == 1) {
- int freecount = 0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- do {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- freecount += rec.ir_freecount;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- } while (i == 1);
+ error = xfs_check_agi_freecount(cur, agi);
+ if (error)
+ goto error0;
- ASSERT(freecount == be32_to_cpu(agi->agi_freecount) ||
- XFS_FORCED_SHUTDOWN(mp));
- }
-#endif
/*
* If in the same a.g. as the parent, try to get near the parent.
*/
@@ -951,25 +977,11 @@ nextag:
down_read(&mp->m_peraglock);
mp->m_perag[tagno].pagi_freecount--;
up_read(&mp->m_peraglock);
-#ifdef DEBUG
- if (cur->bc_nlevels == 1) {
- int freecount = 0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
- goto error0;
- do {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- freecount += rec.ir_freecount;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- } while (i == 1);
- ASSERT(freecount == be32_to_cpu(agi->agi_freecount) ||
- XFS_FORCED_SHUTDOWN(mp));
- }
-#endif
+ error = xfs_check_agi_freecount(cur, agi);
+ if (error)
+ goto error0;
+
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
*inop = ino;
@@ -1060,26 +1072,11 @@ xfs_difree(
* Initialize the cursor.
*/
cur = xfs_inobt_init_cursor(mp, tp, agbp, agno);
-#ifdef DEBUG
- if (cur->bc_nlevels == 1) {
- int freecount = 0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
- goto error0;
- do {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- if (i) {
- freecount += rec.ir_freecount;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- }
- } while (i == 1);
- ASSERT(freecount == be32_to_cpu(agi->agi_freecount) ||
- XFS_FORCED_SHUTDOWN(mp));
- }
-#endif
+ error = xfs_check_agi_freecount(cur, agi);
+ if (error)
+ goto error0;
+
/*
* Look for the entry describing this inode.
*/
@@ -1165,26 +1162,10 @@ xfs_difree(
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
}
-#ifdef DEBUG
- if (cur->bc_nlevels == 1) {
- int freecount = 0;
+ error = xfs_check_agi_freecount(cur, agi);
+ if (error)
+ goto error0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
- goto error0;
- do {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- if (i) {
- freecount += rec.ir_freecount;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- }
- } while (i == 1);
- ASSERT(freecount == be32_to_cpu(agi->agi_freecount) ||
- XFS_FORCED_SHUTDOWN(mp));
- }
-#endif
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
return 0;
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 5/7] xfs: untangle xfs_dialloc
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
` (3 preceding siblings ...)
2009-05-12 21:16 ` [PATCH 4/7] xfs: factor out debug checks from xfs_dialloc and xfs_difree Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 6/7] xfs: rationalize xfs_inobt_lookup* Christoph Hellwig
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-dialloc-untangle --]
[-- Type: text/plain, Size: 9191 bytes --]
Clarify the control flow in xfs_dialloc. Factor out a helper to go to the
next node from the current one and improve the control flow by expanding
composite if statements and using gotos.
The xfs_ialloc_next_rec helpers is borrowed from Dave Chinners dynamic
allocation policy patches.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-02-25 18:39:54.179498785 +0100
+++ xfs/fs/xfs/xfs_ialloc.c 2009-02-25 20:16:04.395799709 +0100
@@ -589,6 +589,37 @@ nextag:
}
}
+/*
+ * Try to retrieve the next record to the left/right from the current one.
+ */
+STATIC int
+xfs_ialloc_next_rec(
+ struct xfs_btree_cur *cur,
+ xfs_inobt_rec_incore_t *rec,
+ int *done,
+ int left)
+{
+ int error;
+ int i;
+
+ if (left)
+ error = xfs_btree_decrement(cur, 0, &i);
+ else
+ error = xfs_btree_increment(cur, 0, &i);
+
+ if (error)
+ return error;
+ *done = !i;
+ if (i) {
+ error = xfs_inobt_get_rec(cur, rec, &i);
+ if (error)
+ return error;
+ XFS_WANT_CORRUPTED_RETURN(i == 1);
+ }
+
+ return 0;
+}
+
/*
* Visible inode allocation functions.
@@ -644,8 +675,8 @@ xfs_dialloc(
int j; /* result code */
xfs_mount_t *mp; /* file system mount structure */
int offset; /* index of inode in chunk */
- xfs_agino_t pagino; /* parent's a.g. relative inode # */
- xfs_agnumber_t pagno; /* parent's allocation group number */
+ xfs_agino_t pagino; /* parent's AG relative inode # */
+ xfs_agnumber_t pagno; /* parent's AG number */
xfs_inobt_rec_incore_t rec; /* inode allocation record */
xfs_agnumber_t tagno; /* testing allocation group number */
xfs_btree_cur_t *tcur; /* temp cursor */
@@ -781,186 +812,138 @@ nextag:
goto error0;
/*
- * If in the same a.g. as the parent, try to get near the parent.
+ * If in the same AG as the parent, try to get near the parent.
*/
if (pagno == agno) {
- if ((error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i)))
+ int doneleft; /* done, to the left */
+ int doneright; /* done, to the right */
+
+ error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_RETURN(i == 1);
+
+ error = xfs_inobt_get_rec(cur, &rec, &j);
+ if (error)
goto error0;
- if (i != 0 &&
- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
- j == 1 &&
- rec.ir_freecount > 0) {
+ XFS_WANT_CORRUPTED_RETURN(j == 1);
+
+ if (rec.ir_freecount > 0) {
/*
* Found a free inode in the same chunk
- * as parent, done.
+ * as the parent, done.
*/
+ goto alloc_inode;
}
+
+
/*
- * In the same a.g. as parent, but parent's chunk is full.
+ * In the same AG as parent, but parent's chunk is full.
*/
- else {
- int doneleft; /* done, to the left */
- int doneright; /* done, to the right */
- if (error)
- goto error0;
- ASSERT(i == 1);
- ASSERT(j == 1);
- /*
- * Duplicate the cursor, search left & right
- * simultaneously.
- */
- if ((error = xfs_btree_dup_cursor(cur, &tcur)))
- goto error0;
- /*
- * Search left with tcur, back up 1 record.
- */
- if ((error = xfs_btree_decrement(tcur, 0, &i)))
- goto error1;
- doneleft = !i;
- if (!doneleft) {
- error = xfs_inobt_get_rec(tcur, &trec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+ /* duplicate the cursor, search left & right simultaneously */
+ error = xfs_btree_dup_cursor(cur, &tcur);
+ if (error)
+ goto error0;
+
+ /* search left with tcur, back up 1 record */
+ error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
+ if (error)
+ goto error1;
+
+ /* search right with cur, go forward 1 record. */
+ error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
+ if (error)
+ goto error1;
+
+ /*
+ * Loop until we find an inode chunk with a free inode.
+ */
+ while (!doneleft || !doneright) {
+ int useleft; /* using left inode chunk this time */
+
+ /* figure out the closer block if both are valid. */
+ if (!doneleft && !doneright) {
+ useleft = pagino -
+ (trec.ir_startino + XFS_INODES_PER_CHUNK - 1) <
+ rec.ir_startino - pagino;
+ } else {
+ useleft = !doneleft;
}
- /*
- * Search right with cur, go forward 1 record.
- */
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error1;
- doneright = !i;
- if (!doneright) {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+
+ /* free inodes to the left? */
+ if (useleft && trec.ir_freecount) {
+ rec = trec;
+ xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+ cur = tcur;
+ goto alloc_inode;
}
- /*
- * Loop until we find the closest inode chunk
- * with a free one.
- */
- while (!doneleft || !doneright) {
- int useleft; /* using left inode
- chunk this time */
- /*
- * Figure out which block is closer,
- * if both are valid.
- */
- if (!doneleft && !doneright)
- useleft =
- pagino -
- (trec.ir_startino +
- XFS_INODES_PER_CHUNK - 1) <
- rec.ir_startino - pagino;
- else
- useleft = !doneleft;
- /*
- * If checking the left, does it have
- * free inodes?
- */
- if (useleft && trec.ir_freecount) {
- /*
- * Yes, set it up as the chunk to use.
- */
- rec = trec;
- xfs_btree_del_cursor(cur,
- XFS_BTREE_NOERROR);
- cur = tcur;
- break;
- }
- /*
- * If checking the right, does it have
- * free inodes?
- */
- if (!useleft && rec.ir_freecount) {
- /*
- * Yes, it's already set up.
- */
- xfs_btree_del_cursor(tcur,
- XFS_BTREE_NOERROR);
- break;
- }
- /*
- * If used the left, get another one
- * further left.
- */
- if (useleft) {
- if ((error = xfs_btree_decrement(tcur, 0,
- &i)))
- goto error1;
- doneleft = !i;
- if (!doneleft) {
- error = xfs_inobt_get_rec(
- tcur, &trec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1,
- error1);
- }
- }
- /*
- * If used the right, get another one
- * further right.
- */
- else {
- if ((error = xfs_btree_increment(cur, 0,
- &i)))
- goto error1;
- doneright = !i;
- if (!doneright) {
- error = xfs_inobt_get_rec(
- cur, &rec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1,
- error1);
- }
- }
+ /* free inodes to the right? */
+ if (!useleft && rec.ir_freecount) {
+ xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+ goto alloc_inode;
+ }
+
+ /* get next record to check */
+ if (useleft) {
+ error = xfs_ialloc_next_rec(tcur, &trec,
+ &doneleft, 1);
+ } else {
+ error = xfs_ialloc_next_rec(cur, &rec,
+ &doneright, 0);
}
- ASSERT(!doneleft || !doneright);
}
+ ASSERT(!doneleft || !doneright);
}
+
/*
- * In a different a.g. from the parent.
+ * In a different AG from the parent.
* See if the most recently allocated block has any free.
*/
else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
- if ((error = xfs_inobt_lookup_eq(cur,
- be32_to_cpu(agi->agi_newino), 0, 0, &i)))
+ error = xfs_inobt_lookup_eq(cur, be32_to_cpu(agi->agi_newino),
+ 0, 0, &i);
+ if (error)
goto error0;
- if (i == 1 &&
- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
- j == 1 &&
- rec.ir_freecount > 0) {
- /*
- * The last chunk allocated in the group still has
- * a free inode.
- */
+
+ if (i == 1) {
+ error = xfs_inobt_get_rec(cur, &rec, &j);
+ if (error)
+ goto error0;
+
+ if (j == 1 && rec.ir_freecount > 0) {
+ /*
+ * The last chunk allocated in the group
+ * still has a free inode.
+ */
+ goto alloc_inode;
+ }
}
+
/*
- * None left in the last group, search the whole a.g.
+ * None left in the last group, search the whole AG
*/
- else {
+ error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+ for (;;) {
+ error = xfs_inobt_get_rec(cur, &rec, &i);
if (error)
goto error0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+ if (rec.ir_freecount > 0)
+ break;
+ error = xfs_btree_increment(cur, 0, &i);
+ if (error)
goto error0;
- ASSERT(i == 1);
- for (;;) {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- if (rec.ir_freecount > 0)
- break;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- }
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
}
}
+
+alloc_inode:
offset = xfs_ialloc_find_free(&rec.ir_free);
ASSERT(offset >= 0);
ASSERT(offset < XFS_INODES_PER_CHUNK);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 6/7] xfs: rationalize xfs_inobt_lookup*
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
` (4 preceding siblings ...)
2009-05-12 21:16 ` [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-05-12 21:16 ` [PATCH 7/7] xfs: speed up free inode search Christoph Hellwig
2009-08-24 15:30 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-cleanup-inobt-lookup --]
[-- Type: text/plain, Size: 7855 bytes --]
Currenly we have a xfs_inobt_lookup* variant for each comparism direction,
and all these get all three fields of the inobt records passed, while the
common case is just looking for the inode number and we have only marginally
more callers than xfs_inobt_lookup* variants.
So opencode a direct call to xfs_btree_lookup for the single case where we
need all fields, and replace xfs_inobt_lookup* with a xfs_inobt_looku that
just takes the inode number and the direction for all other callers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-02-25 20:16:04.395799709 +0100
+++ xfs/fs/xfs/xfs_ialloc.c 2009-02-25 20:16:10.253670750 +0100
@@ -57,56 +57,19 @@ xfs_ialloc_cluster_alignment(
}
/*
- * Lookup the record equal to ino in the btree given by cur.
+ * Lookup a record by ino in the btree given by cur.
*/
STATIC int /* error */
-xfs_inobt_lookup_eq(
+xfs_inobt_lookup(
struct xfs_btree_cur *cur, /* btree cursor */
xfs_agino_t ino, /* starting inode of chunk */
- __int32_t fcnt, /* free inode count */
- xfs_inofree_t free, /* free inode mask */
+ xfs_lookup_t dir, /* <=, >=, == */
int *stat) /* success/failure */
{
cur->bc_rec.i.ir_startino = ino;
- cur->bc_rec.i.ir_freecount = fcnt;
- cur->bc_rec.i.ir_free = free;
- return xfs_btree_lookup(cur, XFS_LOOKUP_EQ, stat);
-}
-
-/*
- * Lookup the first record greater than or equal to ino
- * in the btree given by cur.
- */
-int /* error */
-xfs_inobt_lookup_ge(
- struct xfs_btree_cur *cur, /* btree cursor */
- xfs_agino_t ino, /* starting inode of chunk */
- __int32_t fcnt, /* free inode count */
- xfs_inofree_t free, /* free inode mask */
- int *stat) /* success/failure */
-{
- cur->bc_rec.i.ir_startino = ino;
- cur->bc_rec.i.ir_freecount = fcnt;
- cur->bc_rec.i.ir_free = free;
- return xfs_btree_lookup(cur, XFS_LOOKUP_GE, stat);
-}
-
-/*
- * Lookup the first record less than or equal to ino
- * in the btree given by cur.
- */
-int /* error */
-xfs_inobt_lookup_le(
- struct xfs_btree_cur *cur, /* btree cursor */
- xfs_agino_t ino, /* starting inode of chunk */
- __int32_t fcnt, /* free inode count */
- xfs_inofree_t free, /* free inode mask */
- int *stat) /* success/failure */
-{
- cur->bc_rec.i.ir_startino = ino;
- cur->bc_rec.i.ir_freecount = fcnt;
- cur->bc_rec.i.ir_free = free;
- return xfs_btree_lookup(cur, XFS_LOOKUP_LE, stat);
+ cur->bc_rec.i.ir_freecount = 0;
+ cur->bc_rec.i.ir_free = 0;
+ return xfs_btree_lookup(cur, dir, stat);
}
/*
@@ -162,7 +125,7 @@ xfs_check_agi_freecount(
int error;
int i;
- error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+ error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &i);
if (error)
return error;
@@ -431,13 +394,17 @@ xfs_ialloc_ag_alloc(
for (thisino = newino;
thisino < newino + newlen;
thisino += XFS_INODES_PER_CHUNK) {
- if ((error = xfs_inobt_lookup_eq(cur, thisino,
- XFS_INODES_PER_CHUNK, XFS_INOBT_ALL_FREE, &i))) {
+ cur->bc_rec.i.ir_startino = thisino;
+ cur->bc_rec.i.ir_freecount = XFS_INODES_PER_CHUNK;
+ cur->bc_rec.i.ir_free = XFS_INOBT_ALL_FREE;
+ error = xfs_btree_lookup(cur, XFS_LOOKUP_EQ, &i);
+ if (error) {
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
}
ASSERT(i == 0);
- if ((error = xfs_btree_insert(cur, &i))) {
+ error = xfs_btree_insert(cur, &i);
+ if (error) {
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
}
@@ -818,7 +785,7 @@ nextag:
int doneleft; /* done, to the left */
int doneright; /* done, to the right */
- error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i);
+ error = xfs_inobt_lookup(cur, pagino, XFS_LOOKUP_LE, &i);
if (error)
goto error0;
XFS_WANT_CORRUPTED_RETURN(i == 1);
@@ -902,8 +869,8 @@ nextag:
* See if the most recently allocated block has any free.
*/
else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
- error = xfs_inobt_lookup_eq(cur, be32_to_cpu(agi->agi_newino),
- 0, 0, &i);
+ error = xfs_inobt_lookup(cur, be32_to_cpu(agi->agi_newino),
+ XFS_LOOKUP_EQ, &i);
if (error)
goto error0;
@@ -924,7 +891,7 @@ nextag:
/*
* None left in the last group, search the whole AG
*/
- error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+ error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &i);
if (error)
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
@@ -1063,9 +1030,9 @@ xfs_difree(
/*
* Look for the entry describing this inode.
*/
- if ((error = xfs_inobt_lookup_le(cur, agino, 0, 0, &i))) {
+ if ((error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_LE, &i))) {
cmn_err(CE_WARN,
- "xfs_difree: xfs_inobt_lookup_le returned() an error %d on %s. Returning error.",
+ "xfs_difree: xfs_inobt_lookup returned() an error %d on %s. Returning error.",
error, mp->m_fsname);
goto error0;
}
@@ -1275,10 +1242,10 @@ xfs_imap(
}
cur = xfs_inobt_init_cursor(mp, tp, agbp, agno);
- error = xfs_inobt_lookup_le(cur, agino, 0, 0, &i);
+ error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_LE, &i);
if (error) {
xfs_fs_cmn_err(CE_ALERT, mp, "xfs_imap: "
- "xfs_inobt_lookup_le() failed");
+ "xfs_inobt_lookup() failed");
goto error0;
}
Index: xfs/fs/xfs/xfs_ialloc.h
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.h 2009-02-25 20:11:57.475795139 +0100
+++ xfs/fs/xfs/xfs_ialloc.h 2009-02-25 20:16:10.253670750 +0100
@@ -150,18 +150,10 @@ xfs_ialloc_pagi_init(
xfs_agnumber_t agno); /* allocation group number */
/*
- * Lookup the first record greater than or equal to ino
- * in the btree given by cur.
+ * Lookup a record by ino in the btree given by cur.
*/
-int xfs_inobt_lookup_ge(struct xfs_btree_cur *cur, xfs_agino_t ino,
- __int32_t fcnt, xfs_inofree_t free, int *stat);
-
-/*
- * Lookup the first record less than or equal to ino
- * in the btree given by cur.
- */
-int xfs_inobt_lookup_le(struct xfs_btree_cur *cur, xfs_agino_t ino,
- __int32_t fcnt, xfs_inofree_t free, int *stat);
+int xfs_inobt_lookup(struct xfs_btree_cur *cur, xfs_agino_t ino,
+ xfs_lookup_t dir, int *stat);
/*
* Get the data from the pointed-to record.
Index: xfs/fs/xfs/xfs_itable.c
===================================================================
--- xfs.orig/fs/xfs/xfs_itable.c 2009-02-25 20:11:57.484695574 +0100
+++ xfs/fs/xfs/xfs_itable.c 2009-02-25 20:16:10.254670179 +0100
@@ -444,7 +444,8 @@ xfs_bulkstat(
/*
* Lookup the inode chunk that this inode lives in.
*/
- error = xfs_inobt_lookup_le(cur, agino, 0, 0, &tmp);
+ error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_LE,
+ &tmp);
if (!error && /* no I/O error */
tmp && /* lookup succeeded */
/* got the record, should always work */
@@ -492,7 +493,7 @@ xfs_bulkstat(
/*
* Start of ag. Lookup the first inode chunk.
*/
- error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &tmp);
+ error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &tmp);
icount = 0;
}
/*
@@ -511,8 +512,8 @@ xfs_bulkstat(
if (XFS_AGINO_TO_AGBNO(mp, agino) >=
be32_to_cpu(agi->agi_length))
break;
- error = xfs_inobt_lookup_ge(cur, agino, 0, 0,
- &tmp);
+ error = xfs_inobt_lookup(cur, agino,
+ XFS_LOOKUP_GE, &tmp);
cond_resched();
}
/*
@@ -858,7 +859,8 @@ xfs_inumbers(
continue;
}
cur = xfs_inobt_init_cursor(mp, NULL, agbp, agno);
- error = xfs_inobt_lookup_ge(cur, agino, 0, 0, &tmp);
+ error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_GE,
+ &tmp);
if (error) {
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
cur = NULL;
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 7/7] xfs: speed up free inode search
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
` (5 preceding siblings ...)
2009-05-12 21:16 ` [PATCH 6/7] xfs: rationalize xfs_inobt_lookup* Christoph Hellwig
@ 2009-05-12 21:16 ` Christoph Hellwig
2009-08-24 15:30 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
7 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-05-12 21:16 UTC (permalink / raw)
To: xfs; +Cc: Dave Chinner
[-- Attachment #1: xfs-inode-search --]
[-- Type: text/plain, Size: 6780 bytes --]
From: Dave Chinner <dgc@sgi.com>
Don't search too far - abort if it is outside a certain radius and simply do
a linear search for the first free inode. In AGs with a million inodes this
can speed up allocation speed by 3-4x.
[hch: ported to the xfs_ialloc.c world order]
Signed-off-by: Dave Chinner <dgc@sgi.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-05-12 23:09:37.446784120 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2009-05-12 23:11:53.140784530 +0200
@@ -587,6 +587,30 @@ xfs_ialloc_next_rec(
return 0;
}
+STATIC int
+xfs_ialloc_get_rec(
+ struct xfs_btree_cur *cur,
+ xfs_agino_t agino,
+ xfs_inobt_rec_incore_t *rec,
+ int *done,
+ int left)
+{
+ int error;
+ int i;
+
+ error = xfs_inobt_lookup(cur, agino, XFS_LOOKUP_EQ, &i);
+ if (error)
+ return error;
+ *done = !i;
+ if (i) {
+ error = xfs_inobt_get_rec(cur, rec, &i);
+ if (error)
+ return error;
+ XFS_WANT_CORRUPTED_RETURN(i == 1);
+ }
+
+ return 0;
+}
/*
* Visible inode allocation functions.
@@ -766,6 +790,8 @@ nextag:
*/
agno = tagno;
*IO_agbp = NULL;
+
+ restart_pagno:
cur = xfs_inobt_init_cursor(mp, tp, agbp, be32_to_cpu(agi->agi_seqno));
/*
* If pagino is 0 (this is the root inode allocation) use newino.
@@ -782,8 +808,10 @@ nextag:
* If in the same AG as the parent, try to get near the parent.
*/
if (pagno == agno) {
+ xfs_perag_t *pag = &mp->m_perag[agno];
int doneleft; /* done, to the left */
int doneright; /* done, to the right */
+ int searchdistance = 10;
error = xfs_inobt_lookup(cur, pagino, XFS_LOOKUP_LE, &i);
if (error)
@@ -813,15 +841,33 @@ nextag:
if (error)
goto error0;
- /* search left with tcur, back up 1 record */
- error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
- if (error)
- goto error1;
+ /*
+ * Skip to last blocks looked up if same parent inode.
+ */
+ if (pagino != NULLAGINO &&
+ pag->pagl_pagino == pagino &&
+ pag->pagl_leftrec != NULLAGINO &&
+ pag->pagl_rightrec != NULLAGINO) {
+ error = xfs_ialloc_get_rec(tcur, pag->pagl_leftrec,
+ &trec, &doneleft, 1);
+ if (error)
+ goto error1;
- /* search right with cur, go forward 1 record. */
- error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
- if (error)
- goto error1;
+ error = xfs_ialloc_get_rec(cur, pag->pagl_rightrec,
+ &rec, &doneright, 0);
+ if (error)
+ goto error1;
+ } else {
+ /* search left with tcur, back up 1 record */
+ error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
+ if (error)
+ goto error1;
+
+ /* search right with cur, go forward 1 record. */
+ error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
+ if (error)
+ goto error1;
+ }
/*
* Loop until we find an inode chunk with a free inode.
@@ -829,6 +875,17 @@ nextag:
while (!doneleft || !doneright) {
int useleft; /* using left inode chunk this time */
+ if (!--searchdistance) {
+ /*
+ * Not in range - save last search
+ * location and allocate a new inode
+ */
+ pag->pagl_leftrec = trec.ir_startino;
+ pag->pagl_rightrec = rec.ir_startino;
+ pag->pagl_pagino = pagino;
+ goto newino;
+ }
+
/* figure out the closer block if both are valid. */
if (!doneleft && !doneright) {
useleft = pagino -
@@ -843,12 +900,20 @@ nextag:
rec = trec;
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
cur = tcur;
+
+ pag->pagl_leftrec = trec.ir_startino;
+ pag->pagl_rightrec = rec.ir_startino;
+ pag->pagl_pagino = pagino;
goto alloc_inode;
}
/* free inodes to the right? */
if (!useleft && rec.ir_freecount) {
xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+
+ pag->pagl_leftrec = trec.ir_startino;
+ pag->pagl_rightrec = rec.ir_startino;
+ pag->pagl_pagino = pagino;
goto alloc_inode;
}
@@ -861,14 +926,28 @@ nextag:
&doneright, 0);
}
}
- ASSERT(!doneleft || !doneright);
+
+ /*
+ * We've reached the end of the btree. because
+ * we are only searching a small chunk of the
+ * btree each search, there is obviously free
+ * inodes closer to the parent inode than we
+ * are now. restart the search again.
+ */
+ pag->pagl_pagino = NULLAGINO;
+ pag->pagl_leftrec = NULLAGINO;
+ pag->pagl_rightrec = NULLAGINO;
+ xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+ xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+ goto restart_pagno;
}
/*
* In a different AG from the parent.
* See if the most recently allocated block has any free.
*/
- else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
+newino:
+ if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
error = xfs_inobt_lookup(cur, be32_to_cpu(agi->agi_newino),
XFS_LOOKUP_EQ, &i);
if (error)
@@ -887,27 +966,27 @@ nextag:
goto alloc_inode;
}
}
+ }
- /*
- * None left in the last group, search the whole AG
- */
- error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &i);
+ /*
+ * None left in the last group, search the whole AG
+ */
+ error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+ for (;;) {
+ error = xfs_inobt_get_rec(cur, &rec, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+ if (rec.ir_freecount > 0)
+ break;
+ error = xfs_btree_increment(cur, 0, &i);
if (error)
goto error0;
XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-
- for (;;) {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- if (rec.ir_freecount > 0)
- break;
- error = xfs_btree_increment(cur, 0, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- }
}
alloc_inode:
Index: xfs/fs/xfs/xfs_ag.h
===================================================================
--- xfs.orig/fs/xfs/xfs_ag.h 2009-05-12 23:11:45.736784323 +0200
+++ xfs/fs/xfs/xfs_ag.h 2009-05-12 23:11:53.139784193 +0200
@@ -198,6 +198,15 @@ typedef struct xfs_perag
xfs_agino_t pagi_count; /* number of allocated inodes */
int pagb_count; /* pagb slots in use */
xfs_perag_busy_t *pagb_list; /* unstable blocks */
+
+ /*
+ * Inode allocation search lookup optimisation.
+ * If the pagino matches, the search for new inodes
+ * doesn't need to search the near ones again straight away
+ */
+ xfs_agino_t pagl_pagino;
+ xfs_agino_t pagl_leftrec;
+ xfs_agino_t pagl_rightrec;
#ifdef __KERNEL__
spinlock_t pagb_lock; /* lock for pagb_list */
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] inode allocation cleanups
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
` (6 preceding siblings ...)
2009-05-12 21:16 ` [PATCH 7/7] xfs: speed up free inode search Christoph Hellwig
@ 2009-08-24 15:30 ` Christoph Hellwig
2009-08-25 17:57 ` Alex Elder
7 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2009-08-24 15:30 UTC (permalink / raw)
To: xfs
ping? any reviewers for this?
On Tue, May 12, 2009 at 05:16:07PM -0400, Christoph Hellwig wrote:
>
> This series contains various cleanups and refactorings to xfs_ialloc.c,
> some of them are my own and some are from Dave Chinner.
>
> The last patch is also from Dave and speeds up the search for free inodes
> in allocation groups with lots of inodes. I ported it ontop of all
> the previous patches, which made it a lot cleaner.
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 0/7] inode allocation cleanups
2009-08-24 15:30 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
@ 2009-08-25 17:57 ` Alex Elder
2009-08-25 18:56 ` Christoph Hellwig
0 siblings, 1 reply; 16+ messages in thread
From: Alex Elder @ 2009-08-25 17:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
> ping? any reviewers for this?
I no longer have these in my mailbox, but I've reviewed them
all, and they all look good. Very nice result of getting rid
of duplication and making functions a more manageable size.
One comment on patch 5 below.
-Alex
# [PATCH 1/7] xfs: factor out inode initialisation Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 2/7] xfs: improve xfs_inobt_get_rec prototype Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 3/7] xfs: improve xfs_inobt_update prototype Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 4/7] xfs: factor out debug checks from xfs_dialloc and xfs_difree Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
No cursor cleanup on WANT_CORRUPTED_RETURN() just after "if (pagno == agno)".
(Maybe I'm misreading the patch.) I realize this is still better than the
ASSERT() in place currently, but maybe it should be WANT_CORRUPTED_GOTO(error0)
instead.
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 6/7] xfs: rationalize xfs_inobt_lookup* Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
# [PATCH 7/7] xfs: speed up free inode search Christoph Hellwig
Reviewed-by: Alex Elder <aelder@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] inode allocation cleanups
2009-08-25 17:57 ` Alex Elder
@ 2009-08-25 18:56 ` Christoph Hellwig
2009-08-25 19:12 ` Alex Elder
0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2009-08-25 18:56 UTC (permalink / raw)
To: Alex Elder; +Cc: Christoph Hellwig, xfs
On Tue, Aug 25, 2009 at 12:57:16PM -0500, Alex Elder wrote:
> # [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
> No cursor cleanup on WANT_CORRUPTED_RETURN() just after "if (pagno == agno)".
> (Maybe I'm misreading the patch.) I realize this is still better than the
> ASSERT() in place currently, but maybe it should be WANT_CORRUPTED_GOTO(error0)
> instead.
Good catch, the two in xfs_dialloc should indeed be WANT_CORRUPTED_GOTOs.
For the one in xfs_ialloc_next_rec.
Below is the updated version of the patch, still needs to run through
XFSQA again:
Subject: xfs: untangle xfs_dialloc
From: Christoph Hellwig <hch@lst.de>
Clarify the control flow in xfs_dialloc. Factor out a helper to go to the
next node from the current one and improve the control flow by expanding
composite if statements and using gotos.
The xfs_ialloc_next_rec helper is borrowed from Dave Chinners dynamic
allocation policy patches.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Alex Elder <aelder@sgi.com>
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2009-08-25 15:48:34.426442643 -0300
+++ xfs/fs/xfs/xfs_ialloc.c 2009-08-25 15:54:56.312444853 -0300
@@ -589,6 +589,37 @@ nextag:
}
}
+/*
+ * Try to retrieve the next record to the left/right from the current one.
+ */
+STATIC int
+xfs_ialloc_next_rec(
+ struct xfs_btree_cur *cur,
+ xfs_inobt_rec_incore_t *rec,
+ int *done,
+ int left)
+{
+ int error;
+ int i;
+
+ if (left)
+ error = xfs_btree_decrement(cur, 0, &i);
+ else
+ error = xfs_btree_increment(cur, 0, &i);
+
+ if (error)
+ return error;
+ *done = !i;
+ if (i) {
+ error = xfs_inobt_get_rec(cur, rec, &i);
+ if (error)
+ return error;
+ XFS_WANT_CORRUPTED_RETURN(i == 1);
+ }
+
+ return 0;
+}
+
/*
* Visible inode allocation functions.
@@ -644,8 +675,8 @@ xfs_dialloc(
int j; /* result code */
xfs_mount_t *mp; /* file system mount structure */
int offset; /* index of inode in chunk */
- xfs_agino_t pagino; /* parent's a.g. relative inode # */
- xfs_agnumber_t pagno; /* parent's allocation group number */
+ xfs_agino_t pagino; /* parent's AG relative inode # */
+ xfs_agnumber_t pagno; /* parent's AG number */
xfs_inobt_rec_incore_t rec; /* inode allocation record */
xfs_agnumber_t tagno; /* testing allocation group number */
xfs_btree_cur_t *tcur; /* temp cursor */
@@ -781,186 +812,140 @@ nextag:
goto error0;
/*
- * If in the same a.g. as the parent, try to get near the parent.
+ * If in the same AG as the parent, try to get near the parent.
*/
if (pagno == agno) {
- if ((error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i)))
+ int doneleft; /* done, to the left */
+ int doneright; /* done, to the right */
+
+ error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+ error = xfs_inobt_get_rec(cur, &rec, &j);
+ if (error)
goto error0;
- if (i != 0 &&
- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
- j == 1 &&
- rec.ir_freecount > 0) {
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+ if (rec.ir_freecount > 0) {
/*
* Found a free inode in the same chunk
- * as parent, done.
+ * as the parent, done.
*/
+ goto alloc_inode;
}
+
+
/*
- * In the same a.g. as parent, but parent's chunk is full.
+ * In the same AG as parent, but parent's chunk is full.
*/
- else {
- int doneleft; /* done, to the left */
- int doneright; /* done, to the right */
- if (error)
- goto error0;
- ASSERT(i == 1);
- ASSERT(j == 1);
- /*
- * Duplicate the cursor, search left & right
- * simultaneously.
- */
- if ((error = xfs_btree_dup_cursor(cur, &tcur)))
- goto error0;
- /*
- * Search left with tcur, back up 1 record.
- */
- if ((error = xfs_btree_decrement(tcur, 0, &i)))
- goto error1;
- doneleft = !i;
- if (!doneleft) {
- error = xfs_inobt_get_rec(tcur, &trec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+ /* duplicate the cursor, search left & right simultaneously */
+ error = xfs_btree_dup_cursor(cur, &tcur);
+ if (error)
+ goto error0;
+
+ /* search left with tcur, back up 1 record */
+ error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
+ if (error)
+ goto error1;
+
+ /* search right with cur, go forward 1 record. */
+ error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
+ if (error)
+ goto error1;
+
+ /*
+ * Loop until we find an inode chunk with a free inode.
+ */
+ while (!doneleft || !doneright) {
+ int useleft; /* using left inode chunk this time */
+
+ /* figure out the closer block if both are valid. */
+ if (!doneleft && !doneright) {
+ useleft = pagino -
+ (trec.ir_startino + XFS_INODES_PER_CHUNK - 1) <
+ rec.ir_startino - pagino;
+ } else {
+ useleft = !doneleft;
}
- /*
- * Search right with cur, go forward 1 record.
- */
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error1;
- doneright = !i;
- if (!doneright) {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+
+ /* free inodes to the left? */
+ if (useleft && trec.ir_freecount) {
+ rec = trec;
+ xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+ cur = tcur;
+ goto alloc_inode;
}
- /*
- * Loop until we find the closest inode chunk
- * with a free one.
- */
- while (!doneleft || !doneright) {
- int useleft; /* using left inode
- chunk this time */
- /*
- * Figure out which block is closer,
- * if both are valid.
- */
- if (!doneleft && !doneright)
- useleft =
- pagino -
- (trec.ir_startino +
- XFS_INODES_PER_CHUNK - 1) <
- rec.ir_startino - pagino;
- else
- useleft = !doneleft;
- /*
- * If checking the left, does it have
- * free inodes?
- */
- if (useleft && trec.ir_freecount) {
- /*
- * Yes, set it up as the chunk to use.
- */
- rec = trec;
- xfs_btree_del_cursor(cur,
- XFS_BTREE_NOERROR);
- cur = tcur;
- break;
- }
- /*
- * If checking the right, does it have
- * free inodes?
- */
- if (!useleft && rec.ir_freecount) {
- /*
- * Yes, it's already set up.
- */
- xfs_btree_del_cursor(tcur,
- XFS_BTREE_NOERROR);
- break;
- }
- /*
- * If used the left, get another one
- * further left.
- */
- if (useleft) {
- if ((error = xfs_btree_decrement(tcur, 0,
- &i)))
- goto error1;
- doneleft = !i;
- if (!doneleft) {
- error = xfs_inobt_get_rec(
- tcur, &trec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1,
- error1);
- }
- }
- /*
- * If used the right, get another one
- * further right.
- */
- else {
- if ((error = xfs_btree_increment(cur, 0,
- &i)))
- goto error1;
- doneright = !i;
- if (!doneright) {
- error = xfs_inobt_get_rec(
- cur, &rec, &i);
- if (error)
- goto error1;
- XFS_WANT_CORRUPTED_GOTO(i == 1,
- error1);
- }
- }
+ /* free inodes to the right? */
+ if (!useleft && rec.ir_freecount) {
+ xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+ goto alloc_inode;
}
- ASSERT(!doneleft || !doneright);
+
+ /* get next record to check */
+ if (useleft) {
+ error = xfs_ialloc_next_rec(tcur, &trec,
+ &doneleft, 1);
+ } else {
+ error = xfs_ialloc_next_rec(cur, &rec,
+ &doneright, 0);
+ }
+ if (error)
+ goto error1;
}
+ ASSERT(!doneleft || !doneright);
}
+
/*
- * In a different a.g. from the parent.
+ * In a different AG from the parent.
* See if the most recently allocated block has any free.
*/
else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
- if ((error = xfs_inobt_lookup_eq(cur,
- be32_to_cpu(agi->agi_newino), 0, 0, &i)))
+ error = xfs_inobt_lookup_eq(cur, be32_to_cpu(agi->agi_newino),
+ 0, 0, &i);
+ if (error)
goto error0;
- if (i == 1 &&
- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
- j == 1 &&
- rec.ir_freecount > 0) {
- /*
- * The last chunk allocated in the group still has
- * a free inode.
- */
+
+ if (i == 1) {
+ error = xfs_inobt_get_rec(cur, &rec, &j);
+ if (error)
+ goto error0;
+
+ if (j == 1 && rec.ir_freecount > 0) {
+ /*
+ * The last chunk allocated in the group
+ * still has a free inode.
+ */
+ goto alloc_inode;
+ }
}
+
/*
- * None left in the last group, search the whole a.g.
+ * None left in the last group, search the whole AG
*/
- else {
+ error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+ if (error)
+ goto error0;
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+ for (;;) {
+ error = xfs_inobt_get_rec(cur, &rec, &i);
if (error)
goto error0;
- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+ if (rec.ir_freecount > 0)
+ break;
+ error = xfs_btree_increment(cur, 0, &i);
+ if (error)
goto error0;
- ASSERT(i == 1);
- for (;;) {
- error = xfs_inobt_get_rec(cur, &rec, &i);
- if (error)
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- if (rec.ir_freecount > 0)
- break;
- if ((error = xfs_btree_increment(cur, 0, &i)))
- goto error0;
- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
- }
+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
}
}
+
+alloc_inode:
offset = xfs_ialloc_find_free(&rec.ir_free);
ASSERT(offset >= 0);
ASSERT(offset < XFS_INODES_PER_CHUNK);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 0/7] inode allocation cleanups
2009-08-25 18:56 ` Christoph Hellwig
@ 2009-08-25 19:12 ` Alex Elder
2009-08-26 17:46 ` [PATCH] xfs_showargs() reports group *and* project quotas enabled Alex Elder
2009-08-26 17:58 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
0 siblings, 2 replies; 16+ messages in thread
From: Alex Elder @ 2009-08-25 19:12 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Tuesday, August 25, 2009 1:57 PM
> To: Alex Elder
> Cc: Christoph Hellwig; xfs@oss.sgi.com
> Subject: Re: [PATCH 0/7] inode allocation cleanups
>
>
> On Tue, Aug 25, 2009 at 12:57:16PM -0500, Alex Elder wrote:
> > # [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
> > No cursor cleanup on WANT_CORRUPTED_RETURN() just after
> "if (pagno == agno)".
> > (Maybe I'm misreading the patch.) I realize this is
> still better than the
> > ASSERT() in place currently, but maybe it should be
> WANT_CORRUPTED_GOTO(error0)
> > instead.
>
> Good catch, the two in xfs_dialloc should indeed be
> WANT_CORRUPTED_GOTOs.
> For the one in xfs_ialloc_next_rec.
>
>
> Below is the updated version of the patch, still needs to run through
> XFSQA again:
. . .
I did not review this one as carefully, but I see that you changed
those two spots to WANT_CORRUPTED_GOTO() calls and I assume that's
all that's really changed.
Looks good.
-Alex
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] xfs_showargs() reports group *and* project quotas enabled
2009-08-25 19:12 ` Alex Elder
@ 2009-08-26 17:46 ` Alex Elder
2009-08-26 22:14 ` Christoph Hellwig
2009-08-27 6:53 ` Felix Blyakher
2009-08-26 17:58 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
1 sibling, 2 replies; 16+ messages in thread
From: Alex Elder @ 2009-08-26 17:46 UTC (permalink / raw)
To: xfs
If you enable group or project quotas on an XFS file system, then the
mount table presented through /proc/self/mounts erroneously shows
that both options are in effect for the file system. The root of
the problem is some bad logic in the xfs_showargs() function, which
is used to format the file system type-specific options in effect
for a file system.
The problem originated in this GIT commit:
Move platform specific mount option parse out of core XFS code
Date: 11/22/07
Author: Dave Chinner
SHA1 ID: a67d7c5f5d25d0b13a4dfb182697135b014fa478
For XFS quotas, project and group quota management are mutually
exclusive--only one can be in effect at a time. There are two
parts to managing quotas: aggregating usage information; and
enforcing limits. It is possible to have a quota in effect
(aggregating usage) but not enforced.
These features are recorded on an XFS mount point using these flags:
XFS_PQUOTA_ACCT - Project quotas are aggregated
XFS_GQUOTA_ACCT - Group quotas are aggregated
XFS_OQUOTA_ENFD - Project/group quotas are enforced
The code in error is in fs/xfs/linux-2.6/xfs_super.c:
if (mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
seq_puts(m, "," MNTOPT_PRJQUOTA);
else if (mp->m_qflags & XFS_PQUOTA_ACCT)
seq_puts(m, "," MNTOPT_PQUOTANOENF);
if (mp->m_qflags & (XFS_GQUOTA_ACCT|XFS_OQUOTA_ENFD))
seq_puts(m, "," MNTOPT_GRPQUOTA);
else if (mp->m_qflags & XFS_GQUOTA_ACCT)
seq_puts(m, "," MNTOPT_GQUOTANOENF);
The problem is that XFS_OQUOTA_ENFD will be set in mp->m_qflags
if either group or project quotas are enforced, and as a result
both MNTOPT_PRJQUOTA and MNTOPT_GRPQUOTA will be shown as mount
options.
Signed-off-by: Alex Elder <aelder@sgi.com>
---
fs/xfs/linux-2.6/xfs_super.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
Index: b/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -579,15 +579,19 @@ xfs_showargs(
else if (mp->m_qflags & XFS_UQUOTA_ACCT)
seq_puts(m, "," MNTOPT_UQUOTANOENF);
- if (mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
- seq_puts(m, "," MNTOPT_PRJQUOTA);
- else if (mp->m_qflags & XFS_PQUOTA_ACCT)
- seq_puts(m, "," MNTOPT_PQUOTANOENF);
+ /* Either project or group quotas can be active, not both */
- if (mp->m_qflags & (XFS_GQUOTA_ACCT|XFS_OQUOTA_ENFD))
- seq_puts(m, "," MNTOPT_GRPQUOTA);
- else if (mp->m_qflags & XFS_GQUOTA_ACCT)
- seq_puts(m, "," MNTOPT_GQUOTANOENF);
+ if (mp->m_qflags & XFS_PQUOTA_ACCT) {
+ if (mp->m_qflags & XFS_OQUOTA_ENFD)
+ seq_puts(m, "," MNTOPT_PRJQUOTA);
+ else
+ seq_puts(m, "," MNTOPT_PQUOTANOENF);
+ } else if (mp->m_qflags & XFS_GQUOTA_ACCT) {
+ if (mp->m_qflags & XFS_OQUOTA_ENFD)
+ seq_puts(m, "," MNTOPT_GRPQUOTA);
+ else
+ seq_puts(m, "," MNTOPT_GQUOTANOENF);
+ }
if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
seq_puts(m, "," MNTOPT_NOQUOTA);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/7] inode allocation cleanups
2009-08-25 19:12 ` Alex Elder
2009-08-26 17:46 ` [PATCH] xfs_showargs() reports group *and* project quotas enabled Alex Elder
@ 2009-08-26 17:58 ` Christoph Hellwig
1 sibling, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-08-26 17:58 UTC (permalink / raw)
To: Alex Elder; +Cc: Christoph Hellwig, xfs
On Tue, Aug 25, 2009 at 02:12:50PM -0500, Alex Elder wrote:
> I did not review this one as carefully, but I see that you changed
> those two spots to WANT_CORRUPTED_GOTO() calls and I assume that's
> all that's really changed.
That is one change, the other is making sure we goto to the error1
label for all failures from xfs_ialloc_next_rec - two of them were
missing it.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] xfs_showargs() reports group *and* project quotas enabled
2009-08-26 17:46 ` [PATCH] xfs_showargs() reports group *and* project quotas enabled Alex Elder
@ 2009-08-26 22:14 ` Christoph Hellwig
2009-08-27 6:53 ` Felix Blyakher
1 sibling, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2009-08-26 22:14 UTC (permalink / raw)
To: Alex Elder; +Cc: xfs
On Wed, Aug 26, 2009 at 12:46:12PM -0500, Alex Elder wrote:
> If you enable group or project quotas on an XFS file system, then the
> mount table presented through /proc/self/mounts erroneously shows
> that both options are in effect for the file system. The root of
> the problem is some bad logic in the xfs_showargs() function, which
> is used to format the file system type-specific options in effect
> for a file system.
>
> The problem originated in this GIT commit:
> Move platform specific mount option parse out of core XFS code
> Date: 11/22/07
> Author: Dave Chinner
> SHA1 ID: a67d7c5f5d25d0b13a4dfb182697135b014fa478
>
> For XFS quotas, project and group quota management are mutually
> exclusive--only one can be in effect at a time. There are two
> parts to managing quotas: aggregating usage information; and
> enforcing limits. It is possible to have a quota in effect
> (aggregating usage) but not enforced.
>
> These features are recorded on an XFS mount point using these flags:
> XFS_PQUOTA_ACCT - Project quotas are aggregated
> XFS_GQUOTA_ACCT - Group quotas are aggregated
> XFS_OQUOTA_ENFD - Project/group quotas are enforced
>
> The code in error is in fs/xfs/linux-2.6/xfs_super.c:
>
> if (mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
> seq_puts(m, "," MNTOPT_PRJQUOTA);
> else if (mp->m_qflags & XFS_PQUOTA_ACCT)
> seq_puts(m, "," MNTOPT_PQUOTANOENF);
>
> if (mp->m_qflags & (XFS_GQUOTA_ACCT|XFS_OQUOTA_ENFD))
> seq_puts(m, "," MNTOPT_GRPQUOTA);
> else if (mp->m_qflags & XFS_GQUOTA_ACCT)
> seq_puts(m, "," MNTOPT_GQUOTANOENF);
>
> The problem is that XFS_OQUOTA_ENFD will be set in mp->m_qflags
> if either group or project quotas are enforced, and as a result
> both MNTOPT_PRJQUOTA and MNTOPT_GRPQUOTA will be shown as mount
> options.
Indeed. Thanks for the detailed analysis.
Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Signed-off-by: Alex Elder <aelder@sgi.com>
>
> ---
> fs/xfs/linux-2.6/xfs_super.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> Index: b/fs/xfs/linux-2.6/xfs_super.c
> ===================================================================
> --- a/fs/xfs/linux-2.6/xfs_super.c
> +++ b/fs/xfs/linux-2.6/xfs_super.c
> @@ -579,15 +579,19 @@ xfs_showargs(
> else if (mp->m_qflags & XFS_UQUOTA_ACCT)
> seq_puts(m, "," MNTOPT_UQUOTANOENF);
>
> - if (mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
> - seq_puts(m, "," MNTOPT_PRJQUOTA);
> - else if (mp->m_qflags & XFS_PQUOTA_ACCT)
> - seq_puts(m, "," MNTOPT_PQUOTANOENF);
> + /* Either project or group quotas can be active, not both */
>
> - if (mp->m_qflags & (XFS_GQUOTA_ACCT|XFS_OQUOTA_ENFD))
> - seq_puts(m, "," MNTOPT_GRPQUOTA);
> - else if (mp->m_qflags & XFS_GQUOTA_ACCT)
> - seq_puts(m, "," MNTOPT_GQUOTANOENF);
> + if (mp->m_qflags & XFS_PQUOTA_ACCT) {
> + if (mp->m_qflags & XFS_OQUOTA_ENFD)
> + seq_puts(m, "," MNTOPT_PRJQUOTA);
> + else
> + seq_puts(m, "," MNTOPT_PQUOTANOENF);
> + } else if (mp->m_qflags & XFS_GQUOTA_ACCT) {
> + if (mp->m_qflags & XFS_OQUOTA_ENFD)
> + seq_puts(m, "," MNTOPT_GRPQUOTA);
> + else
> + seq_puts(m, "," MNTOPT_GQUOTANOENF);
> + }
>
> if (!(mp->m_qflags & XFS_ALL_QUOTA_ACCT))
> seq_puts(m, "," MNTOPT_NOQUOTA);
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] xfs_showargs() reports group *and* project quotas enabled
2009-08-26 17:46 ` [PATCH] xfs_showargs() reports group *and* project quotas enabled Alex Elder
2009-08-26 22:14 ` Christoph Hellwig
@ 2009-08-27 6:53 ` Felix Blyakher
1 sibling, 0 replies; 16+ messages in thread
From: Felix Blyakher @ 2009-08-27 6:53 UTC (permalink / raw)
To: Alex Elder; +Cc: xfs
On Aug 26, 2009, at 12:46 PM, Alex Elder wrote:
> If you enable group or project quotas on an XFS file system, then the
> mount table presented through /proc/self/mounts erroneously shows
> that both options are in effect for the file system. The root of
> the problem is some bad logic in the xfs_showargs() function, which
> is used to format the file system type-specific options in effect
> for a file system.
>
> The problem originated in this GIT commit:
> Move platform specific mount option parse out of core XFS code
> Date: 11/22/07
> Author: Dave Chinner
> SHA1 ID: a67d7c5f5d25d0b13a4dfb182697135b014fa478
>
> For XFS quotas, project and group quota management are mutually
> exclusive--only one can be in effect at a time. There are two
> parts to managing quotas: aggregating usage information; and
> enforcing limits. It is possible to have a quota in effect
> (aggregating usage) but not enforced.
>
> These features are recorded on an XFS mount point using these flags:
> XFS_PQUOTA_ACCT - Project quotas are aggregated
> XFS_GQUOTA_ACCT - Group quotas are aggregated
> XFS_OQUOTA_ENFD - Project/group quotas are enforced
>
> The code in error is in fs/xfs/linux-2.6/xfs_super.c:
>
> if (mp->m_qflags & (XFS_PQUOTA_ACCT|XFS_OQUOTA_ENFD))
> seq_puts(m, "," MNTOPT_PRJQUOTA);
> else if (mp->m_qflags & XFS_PQUOTA_ACCT)
> seq_puts(m, "," MNTOPT_PQUOTANOENF);
>
> if (mp->m_qflags & (XFS_GQUOTA_ACCT|XFS_OQUOTA_ENFD))
> seq_puts(m, "," MNTOPT_GRPQUOTA);
> else if (mp->m_qflags & XFS_GQUOTA_ACCT)
> seq_puts(m, "," MNTOPT_GQUOTANOENF);
>
> The problem is that XFS_OQUOTA_ENFD will be set in mp->m_qflags
> if either group or project quotas are enforced, and as a result
> both MNTOPT_PRJQUOTA and MNTOPT_GRPQUOTA will be shown as mount
> options.
Good catch, Alex.
> Signed-off-by: Alex Elder <aelder@sgi.com>
Reviewed-by: Felix Blyakher <felixb@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2009-08-27 6:52 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-12 21:16 [PATCH 0/7] inode allocation cleanups Christoph Hellwig
2009-05-12 21:16 ` [PATCH 1/7] xfs: factor out inode initialisation Christoph Hellwig
2009-05-12 21:16 ` [PATCH 2/7] xfs: improve xfs_inobt_get_rec prototype Christoph Hellwig
2009-05-12 21:16 ` [PATCH 3/7] xfs: improve xfs_inobt_update prototype Christoph Hellwig
2009-05-12 21:16 ` [PATCH 4/7] xfs: factor out debug checks from xfs_dialloc and xfs_difree Christoph Hellwig
2009-05-12 21:16 ` [PATCH 5/7] xfs: untangle xfs_dialloc Christoph Hellwig
2009-05-12 21:16 ` [PATCH 6/7] xfs: rationalize xfs_inobt_lookup* Christoph Hellwig
2009-05-12 21:16 ` [PATCH 7/7] xfs: speed up free inode search Christoph Hellwig
2009-08-24 15:30 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
2009-08-25 17:57 ` Alex Elder
2009-08-25 18:56 ` Christoph Hellwig
2009-08-25 19:12 ` Alex Elder
2009-08-26 17:46 ` [PATCH] xfs_showargs() reports group *and* project quotas enabled Alex Elder
2009-08-26 22:14 ` Christoph Hellwig
2009-08-27 6:53 ` Felix Blyakher
2009-08-26 17:58 ` [PATCH 0/7] inode allocation cleanups Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox