* [PATCH 0/7] inode allocator refactoring V2
@ 2012-07-04 14:54 Christoph Hellwig
2012-07-04 14:54 ` [PATCH 1/7] xfs: remove xfs_ialloc_find_free Christoph Hellwig
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
This series turns the higher level inode allocator upside down.
The biggest change is that we try to operate on the incore perag
structure as much as possible instead of reading the AGI buffer.
I don't have a system to measure the benefit on the large create benchmarks
right now, but even if it's not benefitial it at least greatly cleans up
the code.
Changes since V1:
- minor cleanups noted by Dave
Note that this does not collapse the three passes in xfs_dialloc yet -
I tried it and got deadlocks that I haven't fully understood yet. I plan
to look into them when I get a bit more time.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/7] xfs: remove xfs_ialloc_find_free
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-04 14:54 ` [PATCH 2/7] xfs: split xfs_dialloc Christoph Hellwig
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-kill-xfs_ialloc_find_free --]
[-- Type: text/plain, Size: 1162 bytes --]
This function is entirely trivial and only has one caller, so remove it to
simplify the code.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-06-04 12:39:42.512235078 -0400
+++ xfs/fs/xfs/xfs_ialloc.c 2012-06-04 12:39:44.012235117 -0400
@@ -609,13 +609,6 @@ xfs_ialloc_get_rec(
/*
* Visible inode allocation functions.
*/
-/*
- * Find a free (set) bit in the inode bitmask.
- */
-static inline int xfs_ialloc_find_free(xfs_inofree_t *fp)
-{
- return xfs_lowbit64(*fp);
-}
/*
* Allocate an inode on disk.
@@ -995,7 +988,7 @@ newino:
}
alloc_inode:
- offset = xfs_ialloc_find_free(&rec.ir_free);
+ offset = xfs_lowbit64(rec.ir_free);
ASSERT(offset >= 0);
ASSERT(offset < XFS_INODES_PER_CHUNK);
ASSERT((XFS_AGINO_TO_OFFSET(mp, rec.ir_startino) %
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/7] xfs: split xfs_dialloc
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
2012-07-04 14:54 ` [PATCH 1/7] xfs: remove xfs_ialloc_find_free Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-29 20:55 ` Ben Myers
2012-07-04 14:54 ` [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc Christoph Hellwig
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-split-xfs_dialloc --]
[-- Type: text/plain, Size: 12158 bytes --]
Move the actual allocation once we have selected an allocation group into a
separate helper, and make xfs_dialloc a wrapper around it.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 349 +++++++++++++++++++++++++---------------------------
1 file changed, 174 insertions(+), 175 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:14:21.832445616 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:23:27.395775691 +0200
@@ -607,188 +607,35 @@ xfs_ialloc_get_rec(
}
/*
- * Visible inode allocation functions.
- */
-
-/*
- * Allocate an inode on disk.
- * Mode is used to tell whether the new inode will need space, and whether
- * it is a directory.
+ * Allocate an inode.
*
- * The arguments IO_agbp and alloc_done are defined to work within
- * the constraint of one allocation per transaction.
- * xfs_dialloc() is designed to be called twice if it has to do an
- * allocation to make more free inodes. On the first call,
- * IO_agbp should be set to NULL. If an inode is available,
- * i.e., xfs_dialloc() did not need to do an allocation, an inode
- * number is returned. In this case, IO_agbp would be set to the
- * current ag_buf and alloc_done set to false.
- * If an allocation needed to be done, xfs_dialloc would return
- * the current ag_buf in IO_agbp and set alloc_done to true.
- * The caller should then commit the current transaction, allocate a new
- * transaction, and call xfs_dialloc() again, passing in the previous
- * value of IO_agbp. IO_agbp should be held across the transactions.
- * Since the agbp is locked across the two calls, the second call is
- * guaranteed to have a free inode available.
- *
- * Once we successfully pick an inode its number is returned and the
- * on-disk data structures are updated. The inode itself is not read
- * in, since doing so would break ordering constraints with xfs_reclaim.
+ * The caller selected an AG for us, and made sure that free inodes are
+ * available.
*/
-int
-xfs_dialloc(
- xfs_trans_t *tp, /* transaction pointer */
- xfs_ino_t parent, /* parent inode (directory) */
- umode_t mode, /* mode bits for new inode */
- int okalloc, /* ok to allocate more space */
- xfs_buf_t **IO_agbp, /* in/out ag header's buffer */
- boolean_t *alloc_done, /* true if we needed to replenish
- inode freelist */
- xfs_ino_t *inop) /* inode number allocated */
+STATIC int
+xfs_dialloc_ag(
+ struct xfs_trans *tp,
+ struct xfs_buf *agbp,
+ xfs_ino_t parent,
+ xfs_ino_t *inop)
{
- xfs_agnumber_t agcount; /* number of allocation groups */
- xfs_buf_t *agbp; /* allocation group header's buffer */
- xfs_agnumber_t agno; /* allocation group number */
- xfs_agi_t *agi; /* allocation group header structure */
- xfs_btree_cur_t *cur; /* inode allocation btree cursor */
- int error; /* error return value */
- int i; /* result code */
- int ialloced; /* inode allocation status */
- int noroom = 0; /* no space for inode blk allocation */
- xfs_ino_t ino; /* fs-relative inode to be returned */
- /* REFERENCED */
- 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 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 */
- xfs_inobt_rec_incore_t trec; /* temp inode allocation record */
- struct xfs_perag *pag;
-
+ struct xfs_mount *mp = tp->t_mountp;
+ struct xfs_agi *agi = XFS_BUF_TO_AGI(agbp);
+ xfs_agnumber_t agno = be32_to_cpu(agi->agi_seqno);
+ xfs_agnumber_t pagno = XFS_INO_TO_AGNO(mp, parent);
+ xfs_agino_t pagino = XFS_INO_TO_AGINO(mp, parent);
+ struct xfs_perag *pag;
+ struct xfs_btree_cur *cur, *tcur;
+ struct xfs_inobt_rec_incore rec, trec;
+ xfs_ino_t ino;
+ int error;
+ int offset;
+ int i, j;
- if (*IO_agbp == NULL) {
- /*
- * We do not have an agbp, so select an initial allocation
- * group for inode allocation.
- */
- agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
- /*
- * Couldn't find an allocation group satisfying the
- * criteria, give up.
- */
- if (!agbp) {
- *inop = NULLFSINO;
- return 0;
- }
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- } else {
- /*
- * Continue where we left off before. In this case, we
- * know that the allocation group has free inodes.
- */
- agbp = *IO_agbp;
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
- }
- mp = tp->t_mountp;
- agcount = mp->m_sb.sb_agcount;
- agno = be32_to_cpu(agi->agi_seqno);
- tagno = agno;
- pagno = XFS_INO_TO_AGNO(mp, parent);
- pagino = XFS_INO_TO_AGINO(mp, parent);
-
- /*
- * If we have already hit the ceiling of inode blocks then clear
- * okalloc so we scan all available agi structures for a free
- * inode.
- */
-
- if (mp->m_maxicount &&
- mp->m_sb.sb_icount + XFS_IALLOC_INODES(mp) > mp->m_maxicount) {
- noroom = 1;
- okalloc = 0;
- }
-
- /*
- * Loop until we find an allocation group that either has free inodes
- * or in which we can allocate some inodes. Iterate through the
- * allocation groups upward, wrapping at the end.
- */
- *alloc_done = B_FALSE;
- while (!agi->agi_freecount) {
- /*
- * Don't do anything if we're not supposed to allocate
- * any blocks, just go on to the next ag.
- */
- if (okalloc) {
- /*
- * Try to allocate some new inodes in the allocation
- * group.
- */
- if ((error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced))) {
- xfs_trans_brelse(tp, agbp);
- if (error == ENOSPC) {
- *inop = NULLFSINO;
- return 0;
- } else
- return error;
- }
- if (ialloced) {
- /*
- * We successfully allocated some inodes, return
- * the current context to the caller so that it
- * can commit the current transaction and call
- * us again where we left off.
- */
- ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
- *alloc_done = B_TRUE;
- *IO_agbp = agbp;
- *inop = NULLFSINO;
- return 0;
- }
- }
- /*
- * If it failed, give up on this ag.
- */
- xfs_trans_brelse(tp, agbp);
- /*
- * Go on to the next ag: get its ag header.
- */
-nextag:
- if (++tagno == agcount)
- tagno = 0;
- if (tagno == agno) {
- *inop = NULLFSINO;
- return noroom ? ENOSPC : 0;
- }
- pag = xfs_perag_get(mp, tagno);
- if (pag->pagi_inodeok == 0) {
- xfs_perag_put(pag);
- goto nextag;
- }
- error = xfs_ialloc_read_agi(mp, tp, tagno, &agbp);
- xfs_perag_put(pag);
- if (error)
- goto nextag;
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- }
- /*
- * Here with an allocation group that has a free inode.
- * Reset agno since we may have chosen a new ag in the
- * loop above.
- */
- agno = tagno;
- *IO_agbp = NULL;
pag = xfs_perag_get(mp, agno);
restart_pagno:
- cur = xfs_inobt_init_cursor(mp, tp, agbp, be32_to_cpu(agi->agi_seqno));
+ cur = xfs_inobt_init_cursor(mp, tp, agbp, agno);
/*
* If pagino is 0 (this is the root inode allocation) use newino.
* This must work because we've just allocated some.
@@ -1021,6 +868,158 @@ error0:
}
/*
+ * Allocate an inode on disk.
+ *
+ * Mode is used to tell whether the new inode will need space, and whether it
+ * is a directory.
+ *
+ * This function is designed to be called twice if it has to do an allocation
+ * to make more free inodes. On the first call, *IO_agbp should be set to NULL.
+ * If an inode is available without having to performn an allocation, an inode
+ * number is returned. In this case, *IO_agbp would be NULL. If an allocation
+ * needes to be done, xfs_dialloc would return the current AGI buffer in
+ * *IO_agbp. The caller should then commit the current transaction, allocate a
+ * new transaction, and call xfs_dialloc() again, passing in the previous value
+ * of *IO_agbp. IO_agbp should be held across the transactions. Since the AGI
+ * buffer is locked across the two calls, the second call is guaranteed to have
+ * a free inode available.
+ *
+ * Once we successfully pick an inode its number is returned and the on-disk
+ * data structures are updated. The inode itself is not read in, since doing so
+ * would break ordering constraints with xfs_reclaim.
+ */
+int
+xfs_dialloc(
+ struct xfs_trans *tp,
+ xfs_ino_t parent,
+ umode_t mode,
+ int okalloc,
+ struct xfs_buf **IO_agbp,
+ boolean_t *alloc_done,
+ xfs_ino_t *inop)
+{
+ struct xfs_buf *agbp;
+ xfs_agnumber_t agno;
+ struct xfs_agi *agi;
+ int error;
+ int ialloced;
+ int noroom = 0;
+ struct xfs_mount *mp;
+ xfs_agnumber_t tagno;
+ struct xfs_perag *pag;
+
+ if (*IO_agbp == NULL) {
+ /*
+ * We do not have an agbp, so select an initial allocation
+ * group for inode allocation.
+ */
+ agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
+ /*
+ * Couldn't find an allocation group satisfying the
+ * criteria, give up.
+ */
+ if (!agbp) {
+ *inop = NULLFSINO;
+ return 0;
+ }
+ agi = XFS_BUF_TO_AGI(agbp);
+ ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
+ } else {
+ /*
+ * Continue where we left off before. In this case, we
+ * know that the allocation group has free inodes.
+ */
+ agbp = *IO_agbp;
+ agi = XFS_BUF_TO_AGI(agbp);
+ ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
+ ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
+ }
+ mp = tp->t_mountp;
+ agno = be32_to_cpu(agi->agi_seqno);
+ tagno = agno;
+
+ /*
+ * If we have already hit the ceiling of inode blocks then clear
+ * okalloc so we scan all available agi structures for a free
+ * inode.
+ */
+
+ if (mp->m_maxicount &&
+ mp->m_sb.sb_icount + XFS_IALLOC_INODES(mp) > mp->m_maxicount) {
+ noroom = 1;
+ okalloc = 0;
+ }
+
+ /*
+ * Loop until we find an allocation group that either has free inodes
+ * or in which we can allocate some inodes. Iterate through the
+ * allocation groups upward, wrapping at the end.
+ */
+ *alloc_done = B_FALSE;
+ while (!agi->agi_freecount) {
+ /*
+ * Don't do anything if we're not supposed to allocate
+ * any blocks, just go on to the next ag.
+ */
+ if (okalloc) {
+ /*
+ * Try to allocate some new inodes in the allocation
+ * group.
+ */
+ if ((error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced))) {
+ xfs_trans_brelse(tp, agbp);
+ if (error == ENOSPC) {
+ *inop = NULLFSINO;
+ return 0;
+ } else
+ return error;
+ }
+ if (ialloced) {
+ /*
+ * We successfully allocated some inodes, return
+ * the current context to the caller so that it
+ * can commit the current transaction and call
+ * us again where we left off.
+ */
+ ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
+ *alloc_done = B_TRUE;
+ *IO_agbp = agbp;
+ *inop = NULLFSINO;
+ return 0;
+ }
+ }
+ /*
+ * If it failed, give up on this ag.
+ */
+ xfs_trans_brelse(tp, agbp);
+ /*
+ * Go on to the next ag: get its ag header.
+ */
+nextag:
+ if (++tagno == mp->m_sb.sb_agcount)
+ tagno = 0;
+ if (tagno == agno) {
+ *inop = NULLFSINO;
+ return noroom ? ENOSPC : 0;
+ }
+ pag = xfs_perag_get(mp, tagno);
+ if (pag->pagi_inodeok == 0) {
+ xfs_perag_put(pag);
+ goto nextag;
+ }
+ error = xfs_ialloc_read_agi(mp, tp, tagno, &agbp);
+ xfs_perag_put(pag);
+ if (error)
+ goto nextag;
+ agi = XFS_BUF_TO_AGI(agbp);
+ ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
+ }
+
+ *IO_agbp = NULL;
+ return xfs_dialloc_ag(tp, agbp, parent, inop);
+}
+
+/*
* Free disk inode. Carefully avoids touching the incore inode, all
* manipulations incore are the caller's responsibility.
* The on-disk inode is not changed by this operation, only the
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
2012-07-04 14:54 ` [PATCH 1/7] xfs: remove xfs_ialloc_find_free Christoph Hellwig
2012-07-04 14:54 ` [PATCH 2/7] xfs: split xfs_dialloc Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-26 17:48 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-simplify-xfs_dialloc --]
[-- Type: text/plain, Size: 5314 bytes --]
We can simplify check the IO_agbp pointer for being non-NULL instead of
passing another argument through two layers of function calls.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 3 ---
fs/xfs/xfs_ialloc.h | 2 --
fs/xfs/xfs_inode.c | 5 ++---
fs/xfs/xfs_inode.h | 2 +-
fs/xfs/xfs_utils.c | 17 +++++++----------
5 files changed, 10 insertions(+), 19 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:23:27.000000000 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:23:49.299108893 +0200
@@ -895,7 +895,6 @@ xfs_dialloc(
umode_t mode,
int okalloc,
struct xfs_buf **IO_agbp,
- boolean_t *alloc_done,
xfs_ino_t *inop)
{
struct xfs_buf *agbp;
@@ -955,7 +954,6 @@ xfs_dialloc(
* or in which we can allocate some inodes. Iterate through the
* allocation groups upward, wrapping at the end.
*/
- *alloc_done = B_FALSE;
while (!agi->agi_freecount) {
/*
* Don't do anything if we're not supposed to allocate
@@ -982,7 +980,6 @@ xfs_dialloc(
* us again where we left off.
*/
ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
- *alloc_done = B_TRUE;
*IO_agbp = agbp;
*inop = NULLFSINO;
return 0;
Index: xfs/fs/xfs/xfs_inode.c
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.c 2012-07-02 12:22:53.000000000 +0200
+++ xfs/fs/xfs/xfs_inode.c 2012-07-02 12:23:35.762442307 +0200
@@ -887,7 +887,6 @@ xfs_ialloc(
prid_t prid,
int okalloc,
xfs_buf_t **ialloc_context,
- boolean_t *call_again,
xfs_inode_t **ipp)
{
xfs_ino_t ino;
@@ -902,10 +901,10 @@ xfs_ialloc(
* the on-disk inode to be allocated.
*/
error = xfs_dialloc(tp, pip ? pip->i_ino : 0, mode, okalloc,
- ialloc_context, call_again, &ino);
+ ialloc_context, &ino);
if (error)
return error;
- if (*call_again || ino == NULLFSINO) {
+ if (*ialloc_context || ino == NULLFSINO) {
*ipp = NULL;
return 0;
}
Index: xfs/fs/xfs/xfs_inode.h
===================================================================
--- xfs.orig/fs/xfs/xfs_inode.h 2012-07-02 12:22:53.000000000 +0200
+++ xfs/fs/xfs/xfs_inode.h 2012-07-02 12:23:35.765775640 +0200
@@ -517,7 +517,7 @@ void xfs_inode_free(struct xfs_inode *i
*/
int xfs_ialloc(struct xfs_trans *, xfs_inode_t *, umode_t,
xfs_nlink_t, xfs_dev_t, prid_t, int,
- struct xfs_buf **, boolean_t *, xfs_inode_t **);
+ struct xfs_buf **, xfs_inode_t **);
uint xfs_ip2xflags(struct xfs_inode *);
uint xfs_dic2xflags(struct xfs_dinode *);
Index: xfs/fs/xfs/xfs_utils.c
===================================================================
--- xfs.orig/fs/xfs/xfs_utils.c 2012-07-02 12:22:53.000000000 +0200
+++ xfs/fs/xfs/xfs_utils.c 2012-07-02 12:23:35.765775640 +0200
@@ -65,7 +65,6 @@ xfs_dir_ialloc(
xfs_trans_t *ntp;
xfs_inode_t *ip;
xfs_buf_t *ialloc_context = NULL;
- boolean_t call_again = B_FALSE;
int code;
uint log_res;
uint log_count;
@@ -91,7 +90,7 @@ xfs_dir_ialloc(
* the inode(s) that we've just allocated.
*/
code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid, okalloc,
- &ialloc_context, &call_again, &ip);
+ &ialloc_context, &ip);
/*
* Return an error if we were unable to allocate a new inode.
@@ -102,19 +101,18 @@ xfs_dir_ialloc(
*ipp = NULL;
return code;
}
- if (!call_again && (ip == NULL)) {
+ if (!ialloc_context && !ip) {
*ipp = NULL;
return XFS_ERROR(ENOSPC);
}
/*
- * If call_again is set, then we were unable to get an
+ * If the AGI buffer is non-NULL, then we were unable to get an
* inode in one operation. We need to commit the current
* transaction and call xfs_ialloc() again. It is guaranteed
* to succeed the second time.
*/
- if (call_again) {
-
+ if (ialloc_context) {
/*
* Normally, xfs_trans_commit releases all the locks.
* We call bhold to hang on to the ialloc_context across
@@ -195,7 +193,7 @@ xfs_dir_ialloc(
* this call should always succeed.
*/
code = xfs_ialloc(tp, dp, mode, nlink, rdev, prid,
- okalloc, &ialloc_context, &call_again, &ip);
+ okalloc, &ialloc_context, &ip);
/*
* If we get an error at this point, return to the caller
@@ -206,12 +204,11 @@ xfs_dir_ialloc(
*ipp = NULL;
return code;
}
- ASSERT ((!call_again) && (ip != NULL));
+ ASSERT(!ialloc_context && ip);
} else {
- if (committed != NULL) {
+ if (committed != NULL)
*committed = 0;
- }
}
*ipp = ip;
Index: xfs/fs/xfs/xfs_ialloc.h
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.h 2012-07-02 12:22:53.000000000 +0200
+++ xfs/fs/xfs/xfs_ialloc.h 2012-07-02 12:23:35.765775640 +0200
@@ -75,8 +75,6 @@ xfs_dialloc(
umode_t mode, /* mode bits for new inode */
int okalloc, /* ok to allocate more space */
struct xfs_buf **agbp, /* buf for a.g. inode header */
- boolean_t *alloc_done, /* an allocation was done to replenish
- the free inodes */
xfs_ino_t *inop); /* inode number allocated */
/*
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
` (2 preceding siblings ...)
2012-07-04 14:54 ` [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-26 17:50 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select Christoph Hellwig
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-dialloc-shortcut-second-call --]
[-- Type: text/plain, Size: 2581 bytes --]
In this case we already have selected an AG and know it has free space
beause the buffer lock never got released. Jump directly into xfs_dialloc_ag
and short cut the AG selection loop.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 47 ++++++++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 21 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:23:49.299108893 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:24:18.305775386 +0200
@@ -634,6 +634,10 @@ xfs_dialloc_ag(
pag = xfs_perag_get(mp, agno);
+ ASSERT(pag->pagi_init);
+ ASSERT(pag->pagi_inodeok);
+ ASSERT(pag->pagi_freecount > 0);
+
restart_pagno:
cur = xfs_inobt_init_cursor(mp, tp, agbp, agno);
/*
@@ -907,32 +911,32 @@ xfs_dialloc(
xfs_agnumber_t tagno;
struct xfs_perag *pag;
- if (*IO_agbp == NULL) {
- /*
- * We do not have an agbp, so select an initial allocation
- * group for inode allocation.
- */
- agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
- /*
- * Couldn't find an allocation group satisfying the
- * criteria, give up.
- */
- if (!agbp) {
- *inop = NULLFSINO;
- return 0;
- }
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- } else {
+ if (*IO_agbp) {
/*
- * Continue where we left off before. In this case, we
+ * If the caller passes in a pointer to the AGI buffer,
+ * continue where we left off before. In this case, we
* know that the allocation group has free inodes.
*/
agbp = *IO_agbp;
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
+ goto out_alloc;
}
+
+ /*
+ * We do not have an agbp, so select an initial allocation
+ * group for inode allocation.
+ */
+ agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
+
+ /*
+ * Couldn't find an allocation group satisfying the
+ * criteria, give up.
+ */
+ if (!agbp) {
+ *inop = NULLFSINO;
+ return 0;
+ }
+ agi = XFS_BUF_TO_AGI(agbp);
+
mp = tp->t_mountp;
agno = be32_to_cpu(agi->agi_seqno);
tagno = agno;
@@ -1012,6 +1016,7 @@ nextag:
ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
}
+out_alloc:
*IO_agbp = NULL;
return xfs_dialloc_ag(tp, agbp, parent, inop);
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
` (3 preceding siblings ...)
2012-07-04 14:54 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-26 17:53 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary Christoph Hellwig
2012-07-04 14:54 ` [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc Christoph Hellwig
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-cleanup-xfs_ialloc_ag_select --]
[-- Type: text/plain, Size: 5205 bytes --]
Loop over the in-core perag structures and prefer using pagi_freecount over
going out to the AGI buffer where possible.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 95 ++++++++++++++++++++++++----------------------------
1 file changed, 44 insertions(+), 51 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:24:18.305775386 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:24:50.605775194 +0200
@@ -442,14 +442,13 @@ xfs_ialloc_next_ag(
* Select an allocation group to look for a free inode in, based on the parent
* inode and then mode. Return the allocation group buffer.
*/
-STATIC xfs_buf_t * /* allocation group buffer */
+STATIC xfs_agnumber_t
xfs_ialloc_ag_select(
xfs_trans_t *tp, /* transaction pointer */
xfs_ino_t parent, /* parent directory inode number */
umode_t mode, /* bits set to indicate file type */
int okalloc) /* ok to allocate more space */
{
- xfs_buf_t *agbp; /* allocation group header buffer */
xfs_agnumber_t agcount; /* number of ag's in the filesystem */
xfs_agnumber_t agno; /* current ag number */
int flags; /* alloc buffer locking flags */
@@ -459,6 +458,7 @@ xfs_ialloc_ag_select(
int needspace; /* file mode implies space allocated */
xfs_perag_t *pag; /* per allocation group data */
xfs_agnumber_t pagno; /* parent (starting) ag number */
+ int error;
/*
* Files of these types need at least one block if length > 0
@@ -474,7 +474,9 @@ xfs_ialloc_ag_select(
if (pagno >= agcount)
pagno = 0;
}
+
ASSERT(pagno < agcount);
+
/*
* Loop through allocation groups, looking for one with a little
* free space in it. Note we don't look for free inodes, exactly.
@@ -486,51 +488,45 @@ xfs_ialloc_ag_select(
flags = XFS_ALLOC_FLAG_TRYLOCK;
for (;;) {
pag = xfs_perag_get(mp, agno);
+ if (!pag->pagi_inodeok) {
+ xfs_ialloc_next_ag(mp);
+ goto nextag;
+ }
+
if (!pag->pagi_init) {
- if (xfs_ialloc_read_agi(mp, tp, agno, &agbp)) {
- agbp = NULL;
+ error = xfs_ialloc_pagi_init(mp, tp, agno);
+ if (error)
goto nextag;
- }
- } else
- agbp = NULL;
+ }
- if (!pag->pagi_inodeok) {
- xfs_ialloc_next_ag(mp);
- goto unlock_nextag;
+ if (pag->pagi_freecount) {
+ xfs_perag_put(pag);
+ return agno;
}
- /*
- * Is there enough free space for the file plus a block
- * of inodes (if we need to allocate some)?
- */
- ineed = pag->pagi_freecount ? 0 : XFS_IALLOC_BLOCKS(mp);
- if (ineed && !pag->pagf_init) {
- if (agbp == NULL &&
- xfs_ialloc_read_agi(mp, tp, agno, &agbp)) {
- agbp = NULL;
+ if (!okalloc)
+ goto nextag;
+
+ if (!pag->pagf_init) {
+ error = xfs_alloc_pagf_init(mp, tp, agno, flags);
+ if (error)
goto nextag;
- }
- (void)xfs_alloc_pagf_init(mp, tp, agno, flags);
}
- if (!ineed || pag->pagf_init) {
- if (ineed && !(longest = pag->pagf_longest))
- longest = pag->pagf_flcount > 0;
- if (!ineed ||
- (pag->pagf_freeblks >= needspace + ineed &&
- longest >= ineed &&
- okalloc)) {
- if (agbp == NULL &&
- xfs_ialloc_read_agi(mp, tp, agno, &agbp)) {
- agbp = NULL;
- goto nextag;
- }
- xfs_perag_put(pag);
- return agbp;
- }
+
+ /*
+ * Is there enough free space for the file plus a block of
+ * inodes? (if we need to allocate some)?
+ */
+ ineed = XFS_IALLOC_BLOCKS(mp);
+ longest = pag->pagf_longest;
+ if (!longest)
+ longest = pag->pagf_flcount > 0;
+
+ if (pag->pagf_freeblks >= needspace + ineed &&
+ longest >= ineed) {
+ xfs_perag_put(pag);
+ return agno;
}
-unlock_nextag:
- if (agbp)
- xfs_trans_brelse(tp, agbp);
nextag:
xfs_perag_put(pag);
/*
@@ -538,13 +534,13 @@ nextag:
* down.
*/
if (XFS_FORCED_SHUTDOWN(mp))
- return NULL;
+ return NULLAGNUMBER;
agno++;
if (agno >= agcount)
agno = 0;
if (agno == pagno) {
if (flags == 0)
- return NULL;
+ return NULLAGNUMBER;
flags = 0;
}
}
@@ -901,13 +897,13 @@ xfs_dialloc(
struct xfs_buf **IO_agbp,
xfs_ino_t *inop)
{
+ struct xfs_mount *mp = tp->t_mountp;
struct xfs_buf *agbp;
xfs_agnumber_t agno;
struct xfs_agi *agi;
int error;
int ialloced;
int noroom = 0;
- struct xfs_mount *mp;
xfs_agnumber_t tagno;
struct xfs_perag *pag;
@@ -925,20 +921,17 @@ xfs_dialloc(
* We do not have an agbp, so select an initial allocation
* group for inode allocation.
*/
- agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
-
- /*
- * Couldn't find an allocation group satisfying the
- * criteria, give up.
- */
- if (!agbp) {
+ agno = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
+ if (agno == NULLAGNUMBER) {
*inop = NULLFSINO;
return 0;
}
+
+ error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
+ if (error)
+ return XFS_ERROR(error);
agi = XFS_BUF_TO_AGI(agbp);
- mp = tp->t_mountp;
- agno = be32_to_cpu(agi->agi_seqno);
tagno = agno;
/*
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
` (4 preceding siblings ...)
2012-07-04 14:54 ` [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-26 17:53 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc Christoph Hellwig
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-dialloc-cleanup-ag-loop-2 --]
[-- Type: text/plain, Size: 5008 bytes --]
Refactor the AG selection loop in xfs_dialloc to operate on the in-memory
perag data as much as possible. We only read the AGI buffer once we have
selected an AG to allocate inodes now instead of for every AG considered.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 127 ++++++++++++++++++++++++++++------------------------
1 file changed, 69 insertions(+), 58 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:24:50.000000000 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:25:24.365774992 +0200
@@ -900,11 +900,10 @@ xfs_dialloc(
struct xfs_mount *mp = tp->t_mountp;
struct xfs_buf *agbp;
xfs_agnumber_t agno;
- struct xfs_agi *agi;
int error;
int ialloced;
int noroom = 0;
- xfs_agnumber_t tagno;
+ xfs_agnumber_t start_agno;
struct xfs_perag *pag;
if (*IO_agbp) {
@@ -921,25 +920,17 @@ xfs_dialloc(
* We do not have an agbp, so select an initial allocation
* group for inode allocation.
*/
- agno = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
- if (agno == NULLAGNUMBER) {
+ start_agno = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
+ if (start_agno == NULLAGNUMBER) {
*inop = NULLFSINO;
return 0;
}
- error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
- if (error)
- return XFS_ERROR(error);
- agi = XFS_BUF_TO_AGI(agbp);
-
- tagno = agno;
-
/*
* If we have already hit the ceiling of inode blocks then clear
* okalloc so we scan all available agi structures for a free
* inode.
*/
-
if (mp->m_maxicount &&
mp->m_sb.sb_icount + XFS_IALLOC_INODES(mp) > mp->m_maxicount) {
noroom = 1;
@@ -951,67 +942,87 @@ xfs_dialloc(
* or in which we can allocate some inodes. Iterate through the
* allocation groups upward, wrapping at the end.
*/
- while (!agi->agi_freecount) {
- /*
- * Don't do anything if we're not supposed to allocate
- * any blocks, just go on to the next ag.
- */
- if (okalloc) {
- /*
- * Try to allocate some new inodes in the allocation
- * group.
- */
- if ((error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced))) {
- xfs_trans_brelse(tp, agbp);
- if (error == ENOSPC) {
- *inop = NULLFSINO;
- return 0;
- } else
- return error;
- }
- if (ialloced) {
- /*
- * We successfully allocated some inodes, return
- * the current context to the caller so that it
- * can commit the current transaction and call
- * us again where we left off.
- */
- ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
- *IO_agbp = agbp;
- *inop = NULLFSINO;
- return 0;
- }
+ agno = start_agno;
+ for (;;) {
+ pag = xfs_perag_get(mp, agno);
+ if (!pag->pagi_inodeok) {
+ xfs_ialloc_next_ag(mp);
+ goto nextag;
+ }
+
+ if (!pag->pagi_init) {
+ error = xfs_ialloc_pagi_init(mp, tp, agno);
+ if (error)
+ goto out_error;
}
+
/*
- * If it failed, give up on this ag.
+ * Do a first racy fast path check if this AG is usable.
*/
- xfs_trans_brelse(tp, agbp);
+ if (!pag->pagi_freecount && !okalloc)
+ goto nextag;
+
+ error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
+ if (error)
+ goto out_error;
+
/*
- * Go on to the next ag: get its ag header.
+ * Once the AGI has been read in we have to recheck
+ * pagi_freecount with the AGI buffer lock held.
*/
-nextag:
- if (++tagno == mp->m_sb.sb_agcount)
- tagno = 0;
- if (tagno == agno) {
+ if (pag->pagi_freecount) {
+ xfs_perag_put(pag);
+ goto out_alloc;
+ }
+
+ if (!okalloc) {
+ xfs_trans_brelse(tp, agbp);
+ goto nextag;
+ }
+
+ error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced);
+ if (error) {
+ xfs_trans_brelse(tp, agbp);
+
+ if (error != ENOSPC)
+ goto out_error;
+
+ xfs_perag_put(pag);
*inop = NULLFSINO;
- return noroom ? ENOSPC : 0;
+ return 0;
}
- pag = xfs_perag_get(mp, tagno);
- if (pag->pagi_inodeok == 0) {
+
+ if (ialloced) {
+ /*
+ * We successfully allocated some inodes, return
+ * the current context to the caller so that it
+ * can commit the current transaction and call
+ * us again where we left off.
+ */
+ ASSERT(pag->pagi_freecount > 0);
xfs_perag_put(pag);
- goto nextag;
+
+ *IO_agbp = agbp;
+ *inop = NULLFSINO;
+ return 0;
}
- error = xfs_ialloc_read_agi(mp, tp, tagno, &agbp);
+
+nextag:
xfs_perag_put(pag);
- if (error)
- goto nextag;
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
+ if (++agno == mp->m_sb.sb_agcount)
+ agno = 0;
+ if (agno == start_agno) {
+ *inop = NULLFSINO;
+ return noroom ? ENOSPC : 0;
+ }
}
out_alloc:
*IO_agbp = NULL;
return xfs_dialloc_ag(tp, agbp, parent, inop);
+out_error:
+ xfs_perag_put(pag);
+ return XFS_ERROR(error);
}
/*
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
` (5 preceding siblings ...)
2012-07-04 14:54 ` [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary Christoph Hellwig
@ 2012-07-04 14:54 ` Christoph Hellwig
2012-07-26 17:47 ` Mark Tinguely
6 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-07-04 14:54 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-dialloc-factor-ag_selection-2 --]
[-- Type: text/plain, Size: 6489 bytes --]
Both function contain the same basic loop over all AGs. Merge the two
by creating three passes in the loop instead of duplicating the code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 179 +++++++++++++++-------------------------------------
1 file changed, 55 insertions(+), 124 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-07-02 12:25:24.365774992 +0200
+++ xfs/fs/xfs/xfs_ialloc.c 2012-07-02 12:26:14.325774734 +0200
@@ -439,114 +439,6 @@ xfs_ialloc_next_ag(
}
/*
- * Select an allocation group to look for a free inode in, based on the parent
- * inode and then mode. Return the allocation group buffer.
- */
-STATIC xfs_agnumber_t
-xfs_ialloc_ag_select(
- xfs_trans_t *tp, /* transaction pointer */
- xfs_ino_t parent, /* parent directory inode number */
- umode_t mode, /* bits set to indicate file type */
- int okalloc) /* ok to allocate more space */
-{
- xfs_agnumber_t agcount; /* number of ag's in the filesystem */
- xfs_agnumber_t agno; /* current ag number */
- int flags; /* alloc buffer locking flags */
- xfs_extlen_t ineed; /* blocks needed for inode allocation */
- xfs_extlen_t longest = 0; /* longest extent available */
- xfs_mount_t *mp; /* mount point structure */
- int needspace; /* file mode implies space allocated */
- xfs_perag_t *pag; /* per allocation group data */
- xfs_agnumber_t pagno; /* parent (starting) ag number */
- int error;
-
- /*
- * Files of these types need at least one block if length > 0
- * (and they won't fit in the inode, but that's hard to figure out).
- */
- needspace = S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode);
- mp = tp->t_mountp;
- agcount = mp->m_maxagi;
- if (S_ISDIR(mode))
- pagno = xfs_ialloc_next_ag(mp);
- else {
- pagno = XFS_INO_TO_AGNO(mp, parent);
- if (pagno >= agcount)
- pagno = 0;
- }
-
- ASSERT(pagno < agcount);
-
- /*
- * Loop through allocation groups, looking for one with a little
- * free space in it. Note we don't look for free inodes, exactly.
- * Instead, we include whether there is a need to allocate inodes
- * to mean that blocks must be allocated for them,
- * if none are currently free.
- */
- agno = pagno;
- flags = XFS_ALLOC_FLAG_TRYLOCK;
- for (;;) {
- pag = xfs_perag_get(mp, agno);
- if (!pag->pagi_inodeok) {
- xfs_ialloc_next_ag(mp);
- goto nextag;
- }
-
- if (!pag->pagi_init) {
- error = xfs_ialloc_pagi_init(mp, tp, agno);
- if (error)
- goto nextag;
- }
-
- if (pag->pagi_freecount) {
- xfs_perag_put(pag);
- return agno;
- }
-
- if (!okalloc)
- goto nextag;
-
- if (!pag->pagf_init) {
- error = xfs_alloc_pagf_init(mp, tp, agno, flags);
- if (error)
- goto nextag;
- }
-
- /*
- * Is there enough free space for the file plus a block of
- * inodes? (if we need to allocate some)?
- */
- ineed = XFS_IALLOC_BLOCKS(mp);
- longest = pag->pagf_longest;
- if (!longest)
- longest = pag->pagf_flcount > 0;
-
- if (pag->pagf_freeblks >= needspace + ineed &&
- longest >= ineed) {
- xfs_perag_put(pag);
- return agno;
- }
-nextag:
- xfs_perag_put(pag);
- /*
- * No point in iterating over the rest, if we're shutting
- * down.
- */
- if (XFS_FORCED_SHUTDOWN(mp))
- return NULLAGNUMBER;
- agno++;
- if (agno >= agcount)
- agno = 0;
- if (agno == pagno) {
- if (flags == 0)
- return NULLAGNUMBER;
- flags = 0;
- }
- }
-}
-
-/*
* Try to retrieve the next record to the left/right from the current one.
*/
STATIC int
@@ -901,10 +793,10 @@ xfs_dialloc(
struct xfs_buf *agbp;
xfs_agnumber_t agno;
int error;
- int ialloced;
int noroom = 0;
xfs_agnumber_t start_agno;
struct xfs_perag *pag;
+ int pass;
if (*IO_agbp) {
/*
@@ -917,16 +809,6 @@ xfs_dialloc(
}
/*
- * We do not have an agbp, so select an initial allocation
- * group for inode allocation.
- */
- start_agno = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
- if (start_agno == NULLAGNUMBER) {
- *inop = NULLFSINO;
- return 0;
- }
-
- /*
* If we have already hit the ceiling of inode blocks then clear
* okalloc so we scan all available agi structures for a free
* inode.
@@ -938,12 +820,31 @@ xfs_dialloc(
}
/*
- * Loop until we find an allocation group that either has free inodes
- * or in which we can allocate some inodes. Iterate through the
- * allocation groups upward, wrapping at the end.
+ * For directories start with a new allocation groups, for other file
+ * types aim to find an inode close to the parent.
*/
+ if (S_ISDIR(mode)) {
+ start_agno = xfs_ialloc_next_ag(mp);
+ ASSERT(start_agno < mp->m_maxagi);
+ } else {
+ start_agno = XFS_INO_TO_AGNO(mp, parent);
+ if (start_agno >= mp->m_maxagi)
+ start_agno = 0;
+ }
+
+ /*
+ * Loop through allocation groups, looking for one with a little
+ * free space in it. Note we don't look for free inodes, exactly.
+ * Instead, we include whether there is a need to allocate inodes
+ * to mean that blocks must be allocated for them, if none are
+ * currently free.
+ */
+ *inop = NULLFSINO;
agno = start_agno;
+ pass = 0;
for (;;) {
+ int ialloced;
+
pag = xfs_perag_get(mp, agno);
if (!pag->pagi_inodeok) {
xfs_ialloc_next_ag(mp);
@@ -980,6 +881,33 @@ xfs_dialloc(
goto nextag;
}
+ if (!pag->pagf_init) {
+ int flags = pass ? 0 : XFS_ALLOC_FLAG_TRYLOCK;
+
+ error = xfs_alloc_pagf_init(mp, tp, agno, flags);
+ if (error)
+ goto out_error;
+ }
+
+ if (pass < 2) {
+ /*
+ * Is there enough free space for the file plus a block
+ * of inodes?
+ */
+ xfs_extlen_t longest = pag->pagf_longest;
+ int needspace =
+ S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode);
+
+ if (!longest)
+ longest = pag->pagf_flcount > 0;
+
+ if (pag->pagf_freeblks <
+ XFS_IALLOC_BLOCKS(mp) + needspace)
+ goto nextag;
+ if (longest < XFS_IALLOC_BLOCKS(mp))
+ goto nextag;
+ }
+
error = xfs_ialloc_ag_alloc(tp, agbp, &ialloced);
if (error) {
xfs_trans_brelse(tp, agbp);
@@ -1012,8 +940,11 @@ nextag:
if (++agno == mp->m_sb.sb_agcount)
agno = 0;
if (agno == start_agno) {
- *inop = NULLFSINO;
- return noroom ? ENOSPC : 0;
+ if (pass == 2) {
+ *inop = NULLFSINO;
+ return noroom ? ENOSPC : 0;
+ }
+ pass++;
}
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc
2012-07-04 14:54 ` [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc Christoph Hellwig
@ 2012-07-26 17:47 ` Mark Tinguely
2012-07-30 17:21 ` Mark Tinguely
0 siblings, 1 reply; 17+ messages in thread
From: Mark Tinguely @ 2012-07-26 17:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
+ if (pass < 2) {
+ /*
+ * Is there enough free space for the file plus a block
+ * of inodes?
+ */
+ xfs_extlen_t longest = pag->pagf_longest;
+ int needspace =
+ S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode);
+
+ if (!longest)
+ longest = pag->pagf_flcount > 0;
+
+ if (pag->pagf_freeblks <
+ XFS_IALLOC_BLOCKS(mp) + needspace)
+ goto nextag;
^^^^^^^ here
+ if (longest < XFS_IALLOC_BLOCKS(mp))
+ goto nextag;
^^^^^^^ and here
+ }
Isn't the agbp locked from the earlier xfs_ialloc_read_agi()?
Do we want to release them before going on to the next AG?
Thank-you,
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc
2012-07-04 14:54 ` [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc Christoph Hellwig
@ 2012-07-26 17:48 ` Mark Tinguely
0 siblings, 0 replies; 17+ messages in thread
From: Mark Tinguely @ 2012-07-26 17:48 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 07/04/12 09:54, Christoph Hellwig wrote:
> We can simplify check the IO_agbp pointer for being non-NULL instead of
> passing another argument through two layers of function calls.
>
> Reviewed-by: Dave Chinner<dchinner@redhat.com>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
looks good.
Reviewed-by: Mark Tinguely <tinguely@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case
2012-07-04 14:54 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
@ 2012-07-26 17:50 ` Mark Tinguely
0 siblings, 0 replies; 17+ messages in thread
From: Mark Tinguely @ 2012-07-26 17:50 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 07/04/12 09:54, Christoph Hellwig wrote:
> In this case we already have selected an AG and know it has free space
> beause the buffer lock never got released. Jump directly into xfs_dialloc_ag
> and short cut the AG selection loop.
>
> Reviewed-by: Dave Chinner<dchinner@redhat.com>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
>
> ---
Looks good.
Reviewed-by: Mark Tinguely <tinguely@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select
2012-07-04 14:54 ` [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select Christoph Hellwig
@ 2012-07-26 17:53 ` Mark Tinguely
0 siblings, 0 replies; 17+ messages in thread
From: Mark Tinguely @ 2012-07-26 17:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 07/04/12 09:54, Christoph Hellwig wrote:
> Loop over the in-core perag structures and prefer using pagi_freecount over
> going out to the AGI buffer where possible.
>
> Reviewed-by: Dave Chinner<dchinner@redhat.com>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
Looks good
Not your fault, but the "longest" variable is complicated to understand.
Reviewed-by: Mark Tinguely <tinguely@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary
2012-07-04 14:54 ` [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary Christoph Hellwig
@ 2012-07-26 17:53 ` Mark Tinguely
0 siblings, 0 replies; 17+ messages in thread
From: Mark Tinguely @ 2012-07-26 17:53 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 07/04/12 09:54, Christoph Hellwig wrote:
> Refactor the AG selection loop in xfs_dialloc to operate on the in-memory
> perag data as much as possible. We only read the AGI buffer once we have
> selected an AG to allocate inodes now instead of for every AG considered.
>
> Reviewed-by: Dave Chinner<dchinner@redhat.com>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
Looks good.
Reviewed-by: Mark Tinguely <tinguely@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/7] xfs: split xfs_dialloc
2012-07-04 14:54 ` [PATCH 2/7] xfs: split xfs_dialloc Christoph Hellwig
@ 2012-07-29 20:55 ` Ben Myers
0 siblings, 0 replies; 17+ messages in thread
From: Ben Myers @ 2012-07-29 20:55 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Wed, Jul 04, 2012 at 10:54:46AM -0400, Christoph Hellwig wrote:
> Move the actual allocation once we have selected an allocation group into a
> separate helper, and make xfs_dialloc a wrapper around it.
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks fine.
Reviewed-by: Ben Myers <bpm@sgi.com>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc
2012-07-26 17:47 ` Mark Tinguely
@ 2012-07-30 17:21 ` Mark Tinguely
0 siblings, 0 replies; 17+ messages in thread
From: Mark Tinguely @ 2012-07-30 17:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On 07/26/12 12:47, Mark Tinguely wrote:
>
>
> + if (pass < 2) {
> + /*
> + * Is there enough free space for the file plus a block
> + * of inodes?
> + */
> + xfs_extlen_t longest = pag->pagf_longest;
> + int needspace =
> + S_ISDIR(mode) || S_ISREG(mode) || S_ISLNK(mode);
> +
> + if (!longest)
> + longest = pag->pagf_flcount > 0;
> +
> + if (pag->pagf_freeblks <
> + XFS_IALLOC_BLOCKS(mp) + needspace)
> + goto nextag;
> ^^^^^^^ here
>
> + if (longest < XFS_IALLOC_BLOCKS(mp))
> + goto nextag;
>
> ^^^^^^^ and here
> + }
>
> Isn't the agbp locked from the earlier xfs_ialloc_read_agi()?
> Do we want to release them before going on to the next AG?
>
> Thank-you,
>
> --Mark.
>
same line of thought as above:
+ if (!pag->pagf_init) {
+ int flags = pass ? 0 : XFS_ALLOC_FLAG_TRYLOCK;
+
+ error = xfs_alloc_pagf_init(mp, tp, agno, flags);
+ if (error)
+ goto out_error;
+ }
Also, don't we still have the AGI buffer locked when we try to
initialize the AGF? If the AGF initialization fails, do we need to also
unlock the AGI buffer before returning?
Thank-you.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case
2012-06-05 14:46 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
@ 2012-06-18 2:11 ` Dave Chinner
0 siblings, 0 replies; 17+ messages in thread
From: Dave Chinner @ 2012-06-18 2:11 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
On Tue, Jun 05, 2012 at 10:46:51AM -0400, Christoph Hellwig wrote:
> In this case we already have selected an AG and know it has free space
> beause the buffer lock never got released. Jump directly into xfs_dialloc_ag
> and short cut the AG selection loop.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks fine.
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case
2012-06-05 14:46 [PATCH 0/7] inode allocator refactoring Christoph Hellwig
@ 2012-06-05 14:46 ` Christoph Hellwig
2012-06-18 2:11 ` Dave Chinner
0 siblings, 1 reply; 17+ messages in thread
From: Christoph Hellwig @ 2012-06-05 14:46 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-dialloc-shortcut-second-call --]
[-- Type: text/plain, Size: 2571 bytes --]
In this case we already have selected an AG and know it has free space
beause the buffer lock never got released. Jump directly into xfs_dialloc_ag
and short cut the AG selection loop.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/xfs_ialloc.c | 47 ++++++++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 21 deletions(-)
Index: xfs/fs/xfs/xfs_ialloc.c
===================================================================
--- xfs.orig/fs/xfs/xfs_ialloc.c 2012-06-04 12:49:26.040250019 -0400
+++ xfs/fs/xfs/xfs_ialloc.c 2012-06-04 12:49:31.376250156 -0400
@@ -634,6 +634,10 @@ xfs_dialloc_ag(
pag = xfs_perag_get(mp, agno);
+ ASSERT(pag->pagi_init);
+ ASSERT(pag->pagi_inodeok);
+ ASSERT(pag->pagi_freecount > 0);
+
restart_pagno:
cur = xfs_inobt_init_cursor(mp, tp, agbp, agno);
/*
@@ -907,32 +911,32 @@ xfs_dialloc(
xfs_agnumber_t tagno; /* testing allocation group number */
struct xfs_perag *pag;
- if (*IO_agbp == NULL) {
- /*
- * We do not have an agbp, so select an initial allocation
- * group for inode allocation.
- */
- agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
- /*
- * Couldn't find an allocation group satisfying the
- * criteria, give up.
- */
- if (!agbp) {
- *inop = NULLFSINO;
- return 0;
- }
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- } else {
+ if (*IO_agbp) {
/*
- * Continue where we left off before. In this case, we
+ * If the caller passes in a pointer to the AGI buffer,
+ * continue where we left off before. In this case, we
* know that the allocation group has free inodes.
*/
agbp = *IO_agbp;
- agi = XFS_BUF_TO_AGI(agbp);
- ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
- ASSERT(be32_to_cpu(agi->agi_freecount) > 0);
+ goto out_alloc;
}
+
+ /*
+ * We do not have an agbp, so select an initial allocation
+ * group for inode allocation.
+ */
+ agbp = xfs_ialloc_ag_select(tp, parent, mode, okalloc);
+
+ /*
+ * Couldn't find an allocation group satisfying the
+ * criteria, give up.
+ */
+ if (!agbp) {
+ *inop = NULLFSINO;
+ return 0;
+ }
+ agi = XFS_BUF_TO_AGI(agbp);
+
mp = tp->t_mountp;
agno = be32_to_cpu(agi->agi_seqno);
tagno = agno;
@@ -1012,6 +1016,7 @@ nextag:
ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
}
+out_alloc:
*IO_agbp = NULL;
return xfs_dialloc_ag(tp, agbp, parent, inop);
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-07-30 17:21 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-04 14:54 [PATCH 0/7] inode allocator refactoring V2 Christoph Hellwig
2012-07-04 14:54 ` [PATCH 1/7] xfs: remove xfs_ialloc_find_free Christoph Hellwig
2012-07-04 14:54 ` [PATCH 2/7] xfs: split xfs_dialloc Christoph Hellwig
2012-07-29 20:55 ` Ben Myers
2012-07-04 14:54 ` [PATCH 3/7] xfs: remove the alloc_done argument to xfs_dialloc Christoph Hellwig
2012-07-26 17:48 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
2012-07-26 17:50 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 5/7] xfs: refactor xfs_ialloc_ag_select Christoph Hellwig
2012-07-26 17:53 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 6/7] xfs: do not read the AGI buffer in xfs_dialloc until nessecary Christoph Hellwig
2012-07-26 17:53 ` Mark Tinguely
2012-07-04 14:54 ` [PATCH 7/7] xfs: merge xfs_ialloc_ag_select into xfs_dialloc Christoph Hellwig
2012-07-26 17:47 ` Mark Tinguely
2012-07-30 17:21 ` Mark Tinguely
-- strict thread matches above, loose matches on Subject: below --
2012-06-05 14:46 [PATCH 0/7] inode allocator refactoring Christoph Hellwig
2012-06-05 14:46 ` [PATCH 4/7] xfs: add a short cut to xfs_dialloc for the non-NULL agbp case Christoph Hellwig
2012-06-18 2:11 ` Dave Chinner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.