* [PATCH 1/6] XFS: rename xfs_get_perag
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-14 23:11 ` [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code Dave Chinner
` (5 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
xfs_get_perag is really getting the perag that an inode
belongs to based on it's inode number. Convert the use of this
function to just get the perag from a provided ag number.
Use this new function to obtain the per-ag structure when
traversing the per AG inode trees for sync and reclaim.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/linux-2.6/xfs_sync.c | 33 +++++++++++++++++++++------------
fs/xfs/xfs_iget.c | 10 +++++-----
fs/xfs/xfs_inode.c | 8 +++++---
fs/xfs/xfs_mount.h | 8 ++++----
4 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_sync.c b/fs/xfs/linux-2.6/xfs_sync.c
index d895a3a..d8adc33 100644
--- a/fs/xfs/linux-2.6/xfs_sync.c
+++ b/fs/xfs/linux-2.6/xfs_sync.c
@@ -95,13 +95,12 @@ unlock:
STATIC int
xfs_inode_ag_walk(
struct xfs_mount *mp,
- xfs_agnumber_t ag,
+ struct xfs_perag *pag,
int (*execute)(struct xfs_inode *ip,
struct xfs_perag *pag, int flags),
int flags,
int tag)
{
- struct xfs_perag *pag = &mp->m_perag[ag];
uint32_t first_index;
int last_error = 0;
int skipped;
@@ -136,8 +135,6 @@ restart:
delay(1);
goto restart;
}
-
- xfs_put_perag(mp, pag);
return last_error;
}
@@ -154,9 +151,15 @@ xfs_inode_ag_iterator(
xfs_agnumber_t ag;
for (ag = 0; ag < mp->m_sb.sb_agcount; ag++) {
- if (!mp->m_perag[ag].pag_ici_init)
+ struct xfs_perag *pag;
+
+ pag = xfs_perag_get(mp, ag);
+ if (!pag->pag_ici_init) {
+ xfs_perag_put(pag);
continue;
- error = xfs_inode_ag_walk(mp, ag, execute, flags, tag);
+ }
+ error = xfs_inode_ag_walk(mp, pag, execute, flags, tag);
+ xfs_perag_put(pag);
if (error) {
last_error = error;
if (error == EFSCORRUPTED)
@@ -668,25 +671,30 @@ xfs_reclaim_inode(
xfs_inode_t *ip,
int sync_mode)
{
- xfs_perag_t *pag = xfs_get_perag(ip->i_mount, ip->i_ino);
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_perag *pag;
+
- /* The hash lock here protects a thread in xfs_iget_core from
+ /*
+ * The radix tree lock here protects a thread in xfs_iget_core from
* racing with us on linking the inode back with a vnode.
* Once we have the XFS_IRECLAIM flag set it will not touch
* us.
*/
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
write_lock(&pag->pag_ici_lock);
spin_lock(&ip->i_flags_lock);
if (__xfs_iflags_test(ip, XFS_IRECLAIM) ||
!__xfs_iflags_test(ip, XFS_IRECLAIMABLE)) {
spin_unlock(&ip->i_flags_lock);
write_unlock(&pag->pag_ici_lock);
+ xfs_perag_put(pag);
return -EAGAIN;
}
__xfs_iflags_set(ip, XFS_IRECLAIM);
spin_unlock(&ip->i_flags_lock);
write_unlock(&pag->pag_ici_lock);
- xfs_put_perag(ip->i_mount, pag);
+ xfs_perag_put(pag);
/*
* If the inode is still dirty, then flush it out. If the inode
@@ -736,16 +744,17 @@ void
xfs_inode_set_reclaim_tag(
xfs_inode_t *ip)
{
- xfs_mount_t *mp = ip->i_mount;
- xfs_perag_t *pag = xfs_get_perag(mp, ip->i_ino);
+ struct xfs_mount *mp = ip->i_mount;
+ struct xfs_perag *pag;
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
read_lock(&pag->pag_ici_lock);
spin_lock(&ip->i_flags_lock);
__xfs_inode_set_reclaim_tag(pag, ip);
__xfs_iflags_set(ip, XFS_IRECLAIMABLE);
spin_unlock(&ip->i_flags_lock);
read_unlock(&pag->pag_ici_lock);
- xfs_put_perag(mp, pag);
+ xfs_perag_put(pag);
}
void
diff --git a/fs/xfs/xfs_iget.c b/fs/xfs/xfs_iget.c
index 073bb4a..b579aa6 100644
--- a/fs/xfs/xfs_iget.c
+++ b/fs/xfs/xfs_iget.c
@@ -411,7 +411,7 @@ xfs_iget(
return EINVAL;
/* get the perag structure and ensure that it's inode capable */
- pag = xfs_get_perag(mp, ino);
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ino));
if (!pag->pagi_inodeok)
return EINVAL;
ASSERT(pag->pag_ici_init);
@@ -435,7 +435,7 @@ again:
if (error)
goto out_error_or_again;
}
- xfs_put_perag(mp, pag);
+ xfs_perag_put(pag);
*ipp = ip;
@@ -454,7 +454,7 @@ out_error_or_again:
delay(1);
goto again;
}
- xfs_put_perag(mp, pag);
+ xfs_perag_put(pag);
return error;
}
@@ -522,11 +522,11 @@ xfs_ireclaim(
* if it was never added to it because radix_tree_delete can deal
* with that case just fine.
*/
- pag = xfs_get_perag(mp, ip->i_ino);
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
write_lock(&pag->pag_ici_lock);
radix_tree_delete(&pag->pag_ici_root, XFS_INO_TO_AGINO(mp, ip->i_ino));
write_unlock(&pag->pag_ici_lock);
- xfs_put_perag(mp, pag);
+ xfs_perag_put(pag);
/*
* Here we do an (almost) spurious inode lock in order to coordinate
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index b92a4fa..7baf7c0 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1982,8 +1982,9 @@ xfs_ifree_cluster(
xfs_inode_t *ip, **ip_found;
xfs_inode_log_item_t *iip;
xfs_log_item_t *lip;
- xfs_perag_t *pag = xfs_get_perag(mp, inum);
+ struct xfs_perag *pag;
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, inum));
if (mp->m_sb.sb_blocksize >= XFS_INODE_CLUSTER_SIZE(mp)) {
blks_per_cluster = 1;
ninodes = mp->m_sb.sb_inopblock;
@@ -2124,7 +2125,7 @@ xfs_ifree_cluster(
}
kmem_free(ip_found);
- xfs_put_perag(mp, pag);
+ xfs_perag_put(pag);
}
/*
@@ -2711,7 +2712,7 @@ xfs_iflush_cluster(
xfs_buf_t *bp)
{
xfs_mount_t *mp = ip->i_mount;
- xfs_perag_t *pag = xfs_get_perag(mp, ip->i_ino);
+ struct xfs_perag *pag;
unsigned long first_index, mask;
unsigned long inodes_per_cluster;
int ilist_size;
@@ -2722,6 +2723,7 @@ xfs_iflush_cluster(
int bufwasdelwri;
int i;
+ pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
ASSERT(pag->pagi_inodeok);
ASSERT(pag->pag_ici_init);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 1df7e45..f8a68a2 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -386,14 +386,14 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
/*
* perag get/put wrappers for eventual ref counting
*/
-static inline xfs_perag_t *
-xfs_get_perag(struct xfs_mount *mp, xfs_ino_t ino)
+static inline struct xfs_perag *
+xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
{
- return &mp->m_perag[XFS_INO_TO_AGNO(mp, ino)];
+ return &mp->m_perag[agno];
}
static inline void
-xfs_put_perag(struct xfs_mount *mp, xfs_perag_t *pag)
+xfs_perag_put(struct xfs_perag *pag)
{
/* nothing to see here, move along */
}
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
2009-12-14 23:11 ` [PATCH 1/6] XFS: rename xfs_get_perag Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-14 23:11 ` [PATCH 3/6] XFS: Convert filestreams code to use per-ag get/put routines Dave Chinner
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
Start abstracting the perag references so that the indexing of the
structures is not directly coded into all the places that uses the
perag structures. This will allow us to separate the use of the
perag structure and the way it is indexed and hence avoid the known
deadlocks related to growing a busy filesystem.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_alloc.c | 74 ++++++++++++++++++++++++++-------------------
fs/xfs/xfs_alloc_btree.c | 9 ++++-
2 files changed, 50 insertions(+), 33 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 2cf944e..34873ac 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -1800,11 +1800,13 @@ xfs_free_ag_extent(
xfs_agf_t *agf;
xfs_perag_t *pag; /* per allocation group data */
+ pag = xfs_perag_get(mp, agno);
+ pag->pagf_freeblks += len;
+ xfs_perag_put(pag);
+
agf = XFS_BUF_TO_AGF(agbp);
- pag = &mp->m_perag[agno];
be32_add_cpu(&agf->agf_freeblks, len);
xfs_trans_agblocks_delta(tp, len);
- pag->pagf_freeblks += len;
XFS_WANT_CORRUPTED_GOTO(
be32_to_cpu(agf->agf_freeblks) <=
be32_to_cpu(agf->agf_length),
@@ -2110,7 +2112,8 @@ xfs_alloc_get_freelist(
xfs_trans_brelse(tp, agflbp);
if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
agf->agf_flfirst = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, -1);
xfs_trans_agflist_delta(tp, -1);
pag->pagf_flcount--;
@@ -2121,6 +2124,7 @@ xfs_alloc_get_freelist(
pag->pagf_btreeblks++;
logflags |= XFS_AGF_BTREEBLKS;
}
+ xfs_perag_put(pag);
TRACE_MODAGF(NULL, agf, logflags);
xfs_alloc_log_agf(tp, agbp, logflags);
@@ -2218,7 +2222,8 @@ xfs_alloc_put_freelist(
be32_add_cpu(&agf->agf_fllast, 1);
if (be32_to_cpu(agf->agf_fllast) == XFS_AGFL_SIZE(mp))
agf->agf_fllast = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, 1);
xfs_trans_agflist_delta(tp, 1);
pag->pagf_flcount++;
@@ -2229,6 +2234,7 @@ xfs_alloc_put_freelist(
pag->pagf_btreeblks--;
logflags |= XFS_AGF_BTREEBLKS;
}
+ xfs_perag_put(pag);
TRACE_MODAGF(NULL, agf, logflags);
xfs_alloc_log_agf(tp, agbp, logflags);
@@ -2326,7 +2332,7 @@ xfs_alloc_read_agf(
ASSERT(!XFS_BUF_GETERROR(*bpp));
agf = XFS_BUF_TO_AGF(*bpp);
- pag = &mp->m_perag[agno];
+ pag = xfs_perag_get(mp, agno);
if (!pag->pagf_init) {
pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
pag->pagf_btreeblks = be32_to_cpu(agf->agf_btreeblks);
@@ -2353,6 +2359,7 @@ xfs_alloc_read_agf(
be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNTi]));
}
#endif
+ xfs_perag_put(pag);
return 0;
}
@@ -2413,7 +2420,7 @@ xfs_alloc_vextent(
*/
args->agno = XFS_FSB_TO_AGNO(mp, args->fsbno);
down_read(&mp->m_peraglock);
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
args->minleft = 0;
error = xfs_alloc_fix_freelist(args, 0);
args->minleft = minleft;
@@ -2483,7 +2490,7 @@ xfs_alloc_vextent(
*/
down_read(&mp->m_peraglock);
for (;;) {
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
if (no_min) args->minleft = 0;
error = xfs_alloc_fix_freelist(args, flags);
args->minleft = minleft;
@@ -2540,6 +2547,7 @@ xfs_alloc_vextent(
}
}
}
+ xfs_perag_put(args->pag);
}
up_read(&mp->m_peraglock);
if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
@@ -2567,8 +2575,10 @@ xfs_alloc_vextent(
args->len);
#endif
}
+ xfs_perag_put(args->pag);
return 0;
error0:
+ xfs_perag_put(args->pag);
up_read(&mp->m_peraglock);
return error;
}
@@ -2595,7 +2605,7 @@ xfs_free_extent(
ASSERT(args.agno < args.mp->m_sb.sb_agcount);
args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);
down_read(&args.mp->m_peraglock);
- args.pag = &args.mp->m_perag[args.agno];
+ args.pag = xfs_perag_get(args.mp, args.agno);
if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))
goto error0;
#ifdef DEBUG
@@ -2605,6 +2615,7 @@ xfs_free_extent(
#endif
error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);
error0:
+ xfs_perag_put(args.pag);
up_read(&args.mp->m_peraglock);
return error;
}
@@ -2626,15 +2637,15 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
xfs_perag_busy_t *bsy;
+ struct xfs_perag *pag;
int n;
- mp = tp->t_mountp;
- spin_lock(&mp->m_perag[agno].pagb_lock);
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
/* search pagb_list for an open slot */
- for (bsy = mp->m_perag[agno].pagb_list, n = 0;
+ for (bsy = pag->pagb_list, n = 0;
n < XFS_PAGB_NUM_SLOTS;
bsy++, n++) {
if (bsy->busy_tp == NULL) {
@@ -2643,8 +2654,8 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
}
if (n < XFS_PAGB_NUM_SLOTS) {
- bsy = &mp->m_perag[agno].pagb_list[n];
- mp->m_perag[agno].pagb_count++;
+ bsy = &pag->pagb_list[n];
+ pag->pagb_count++;
TRACE_BUSY("xfs_alloc_mark_busy", "got", agno, bno, len, n, tp);
bsy->busy_start = bno;
bsy->busy_length = len;
@@ -2661,7 +2672,8 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_trans_set_sync(tp);
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
void
@@ -2669,24 +2681,24 @@ xfs_alloc_clear_busy(xfs_trans_t *tp,
xfs_agnumber_t agno,
int idx)
{
- xfs_mount_t *mp;
xfs_perag_busy_t *list;
+ struct xfs_perag *pag;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- list = mp->m_perag[agno].pagb_list;
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ list = pag->pagb_list;
ASSERT(idx < XFS_PAGB_NUM_SLOTS);
if (list[idx].busy_tp == tp) {
TRACE_UNBUSY("xfs_alloc_clear_busy", "found", agno, idx, tp);
list[idx].busy_tp = NULL;
- mp->m_perag[agno].pagb_count--;
+ pag->pagb_count--;
} else {
TRACE_UNBUSY("xfs_alloc_clear_busy", "missing", agno, idx, tp);
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
@@ -2700,21 +2712,20 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
+ struct xfs_perag *pag;
xfs_perag_busy_t *bsy;
xfs_agblock_t uend, bend;
xfs_lsn_t lsn;
int cnt;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- cnt = mp->m_perag[agno].pagb_count;
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ cnt = pag->pagb_count;
uend = bno + len - 1;
/* search pagb_list for this slot, skipping open slots */
- for (bsy = mp->m_perag[agno].pagb_list; cnt; bsy++) {
+ for (bsy = pag->pagb_list; cnt; bsy++) {
/*
* (start1,length1) within (start2, length2)
@@ -2738,10 +2749,11 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
if (cnt) {
TRACE_BUSYSEARCH("xfs_alloc_search_busy", "found", agno, bno, len, tp);
lsn = bsy->busy_tp->t_commit_lsn;
- spin_unlock(&mp->m_perag[agno].pagb_lock);
- xfs_log_force(mp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
+ spin_unlock(&pag->pagb_lock);
+ xfs_log_force(tp->t_mountp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
} else {
TRACE_BUSYSEARCH("xfs_alloc_search_busy", "not-found", agno, bno, len, tp);
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
}
+ xfs_perag_put(pag);
}
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index c10c3a2..1c4a71a 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -60,12 +60,14 @@ xfs_allocbt_set_root(
struct xfs_agf *agf = XFS_BUF_TO_AGF(agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
int btnum = cur->bc_btnum;
+ struct xfs_perag *pag = xfs_perag_get(cur->bc_mp, seqno);
ASSERT(ptr->s != 0);
agf->agf_roots[btnum] = ptr->s;
be32_add_cpu(&agf->agf_levels[btnum], inc);
- cur->bc_mp->m_perag[seqno].pagf_levels[btnum] += inc;
+ pag->pagf_levels[btnum] += inc;
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_ROOTS | XFS_AGF_LEVELS);
}
@@ -149,6 +151,7 @@ xfs_allocbt_update_lastrec(
{
struct xfs_agf *agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
+ struct xfs_perag *pag;
__be32 len;
int numrecs;
@@ -192,7 +195,9 @@ xfs_allocbt_update_lastrec(
}
agf->agf_longest = len;
- cur->bc_mp->m_perag[seqno].pagf_longest = be32_to_cpu(len);
+ pag = xfs_perag_get(cur->bc_mp, seqno);
+ pag->pagf_longest = be32_to_cpu(len);
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, cur->bc_private.a.agbp, XFS_AGF_LONGEST);
}
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/6] XFS: Convert filestreams code to use per-ag get/put routines
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
2009-12-14 23:11 ` [PATCH 1/6] XFS: rename xfs_get_perag Dave Chinner
2009-12-14 23:11 ` [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-14 23:11 ` [PATCH 4/6] XFS: convert remaining direct references to m_perag Dave Chinner
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_filestream.c | 19 ++++++++++++-------
fs/xfs/xfs_filestream.h | 27 ++++++++++++++++++++++++---
2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index edf8bdf..08c27ba 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -139,6 +139,7 @@ _xfs_filestream_pick_ag(
int flags,
xfs_extlen_t minlen)
{
+ int streams, max_streams;
int err, trylock, nscan;
xfs_extlen_t longest, free, minfree, maxfree = 0;
xfs_agnumber_t ag, max_ag = NULLAGNUMBER;
@@ -154,15 +155,15 @@ _xfs_filestream_pick_ag(
trylock = XFS_ALLOC_FLAG_TRYLOCK;
for (nscan = 0; 1; nscan++) {
-
- TRACE_AG_SCAN(mp, ag, xfs_filestream_peek_ag(mp, ag));
-
- pag = mp->m_perag + ag;
+ pag = xfs_perag_get(mp, ag);
+ TRACE_AG_SCAN(mp, ag, atomic_read(&pag->pagf_fstrms));
if (!pag->pagf_init) {
err = xfs_alloc_pagf_init(mp, NULL, ag, trylock);
- if (err && !trylock)
+ if (err && !trylock) {
+ xfs_perag_put(pag);
return err;
+ }
}
/* Might fail sometimes during the 1st pass with trylock set. */
@@ -172,6 +173,7 @@ _xfs_filestream_pick_ag(
/* Keep track of the AG with the most free blocks. */
if (pag->pagf_freeblks > maxfree) {
maxfree = pag->pagf_freeblks;
+ max_streams = atomic_read(&pag->pagf_fstrms);
max_ag = ag;
}
@@ -194,6 +196,8 @@ _xfs_filestream_pick_ag(
/* Break out, retaining the reference on the AG. */
free = pag->pagf_freeblks;
+ streams = atomic_read(&pag->pagf_fstrms);
+ xfs_perag_put(pag);
*agp = ag;
break;
}
@@ -201,6 +205,7 @@ _xfs_filestream_pick_ag(
/* Drop the reference on this AG, it's not usable. */
xfs_filestream_put_ag(mp, ag);
next_ag:
+ xfs_perag_put(pag);
/* Move to the next AG, wrapping to AG 0 if necessary. */
if (++ag >= mp->m_sb.sb_agcount)
ag = 0;
@@ -228,6 +233,7 @@ next_ag:
if (max_ag != NULLAGNUMBER) {
xfs_filestream_get_ag(mp, max_ag);
TRACE_AG_PICK1(mp, max_ag, maxfree);
+ streams = max_streams;
free = maxfree;
*agp = max_ag;
break;
@@ -239,8 +245,7 @@ next_ag:
return 0;
}
- TRACE_AG_PICK2(mp, startag, *agp, xfs_filestream_peek_ag(mp, *agp),
- free, nscan, flags);
+ TRACE_AG_PICK2(mp, startag, *agp, streams, free, nscan, flags);
return 0;
}
diff --git a/fs/xfs/xfs_filestream.h b/fs/xfs/xfs_filestream.h
index 4aba67c..58378b2 100644
--- a/fs/xfs/xfs_filestream.h
+++ b/fs/xfs/xfs_filestream.h
@@ -79,12 +79,21 @@ extern ktrace_t *xfs_filestreams_trace_buf;
* the cache that reference per-ag array elements that have since been
* reallocated.
*/
+/*
+ * xfs_filestream_peek_ag is only used in tracing code
+ */
static inline int
xfs_filestream_peek_ag(
xfs_mount_t *mp,
xfs_agnumber_t agno)
{
- return atomic_read(&mp->m_perag[agno].pagf_fstrms);
+ struct xfs_perag *pag;
+ int ret;
+
+ pag = xfs_perag_get(mp, agno);
+ ret = atomic_read(&pag->pagf_fstrms);
+ xfs_perag_put(pag);
+ return ret;
}
static inline int
@@ -92,7 +101,13 @@ xfs_filestream_get_ag(
xfs_mount_t *mp,
xfs_agnumber_t agno)
{
- return atomic_inc_return(&mp->m_perag[agno].pagf_fstrms);
+ struct xfs_perag *pag;
+ int ret;
+
+ pag = xfs_perag_get(mp, agno);
+ ret = atomic_inc_return(&pag->pagf_fstrms);
+ xfs_perag_put(pag);
+ return ret;
}
static inline int
@@ -100,7 +115,13 @@ xfs_filestream_put_ag(
xfs_mount_t *mp,
xfs_agnumber_t agno)
{
- return atomic_dec_return(&mp->m_perag[agno].pagf_fstrms);
+ struct xfs_perag *pag;
+ int ret;
+
+ pag = xfs_perag_get(mp, agno);
+ ret = atomic_dec_return(&pag->pagf_fstrms);
+ xfs_perag_put(pag);
+ return ret;
}
/* allocation selection flags */
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6] XFS: convert remaining direct references to m_perag
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
` (2 preceding siblings ...)
2009-12-14 23:11 ` [PATCH 3/6] XFS: Convert filestreams code to use per-ag get/put routines Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-14 23:11 ` [PATCH 5/6] XFS: Replace per-ag array with a radix tree Dave Chinner
` (2 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
Convert the remaining direct lookups of the per ag structures
to use get/put accesses. Ensure that the loops across AGs and
prior users of the interface balance gets and puts correctly.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_bmap.c | 8 +++++++-
fs/xfs/xfs_ialloc.c | 35 +++++++++++++++++++++++++----------
fs/xfs/xfs_inode.c | 5 ++++-
fs/xfs/xfs_mount.c | 9 ++++++---
4 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 8971fb0..719fa11 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -2781,11 +2781,12 @@ xfs_bmap_btalloc(
startag = ag = 0;
notinit = 0;
down_read(&mp->m_peraglock);
+ pag = xfs_perag_get(mp, ag);
while (blen < ap->alen) {
- pag = &mp->m_perag[ag];
if (!pag->pagf_init &&
(error = xfs_alloc_pagf_init(mp, args.tp,
ag, XFS_ALLOC_FLAG_TRYLOCK))) {
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
return error;
}
@@ -2818,6 +2819,7 @@ xfs_bmap_btalloc(
break;
error = xfs_filestream_new_ag(ap, &ag);
+ xfs_perag_put(pag);
if (error) {
up_read(&mp->m_peraglock);
return error;
@@ -2825,6 +2827,7 @@ xfs_bmap_btalloc(
/* loop again to set 'blen'*/
startag = NULLAGNUMBER;
+ pag = xfs_perag_get(mp, ag);
continue;
}
}
@@ -2832,7 +2835,10 @@ xfs_bmap_btalloc(
ag = 0;
if (ag == startag)
break;
+ xfs_perag_put(pag);
+ pag = xfs_perag_get(mp, ag);
}
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
/*
* Since the above loop did a BUF_TRYLOCK, it is
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index cb907ba..884ee13 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -253,6 +253,7 @@ xfs_ialloc_ag_alloc(
xfs_agino_t thisino; /* current inode number, for loop */
int isaligned = 0; /* inode allocation at stripe unit */
/* boundary */
+ struct xfs_perag *pag;
args.tp = tp;
args.mp = tp->t_mountp;
@@ -383,7 +384,9 @@ xfs_ialloc_ag_alloc(
be32_add_cpu(&agi->agi_count, newlen);
be32_add_cpu(&agi->agi_freecount, newlen);
down_read(&args.mp->m_peraglock);
- args.mp->m_perag[agno].pagi_freecount += newlen;
+ pag = xfs_perag_get(args.mp, agno);
+ pag->pagi_freecount += newlen;
+ xfs_perag_put(pag);
up_read(&args.mp->m_peraglock);
agi->agi_newino = cpu_to_be32(newino);
@@ -488,7 +491,7 @@ xfs_ialloc_ag_select(
flags = XFS_ALLOC_FLAG_TRYLOCK;
down_read(&mp->m_peraglock);
for (;;) {
- pag = &mp->m_perag[agno];
+ pag = xfs_perag_get(mp, agno);
if (!pag->pagi_init) {
if (xfs_ialloc_read_agi(mp, tp, agno, &agbp)) {
agbp = NULL;
@@ -527,6 +530,7 @@ xfs_ialloc_ag_select(
agbp = NULL;
goto nextag;
}
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
return agbp;
}
@@ -535,6 +539,7 @@ unlock_nextag:
if (agbp)
xfs_trans_brelse(tp, agbp);
nextag:
+ xfs_perag_put(pag);
/*
* No point in iterating over the rest, if we're shutting
* down.
@@ -672,6 +677,7 @@ xfs_dialloc(
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;
if (*IO_agbp == NULL) {
@@ -772,11 +778,14 @@ nextag:
return noroom ? ENOSPC : 0;
}
down_read(&mp->m_peraglock);
- if (mp->m_perag[tagno].pagi_inodeok == 0) {
+ pag = xfs_perag_get(mp, tagno);
+ if (pag->pagi_inodeok == 0) {
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
goto nextag;
}
error = xfs_ialloc_read_agi(mp, tp, tagno, &agbp);
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
if (error)
goto nextag;
@@ -790,6 +799,7 @@ nextag:
*/
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));
@@ -808,7 +818,6 @@ 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;
@@ -1007,7 +1016,7 @@ alloc_inode:
be32_add_cpu(&agi->agi_freecount, -1);
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
down_read(&mp->m_peraglock);
- mp->m_perag[tagno].pagi_freecount--;
+ pag->pagi_freecount--;
up_read(&mp->m_peraglock);
error = xfs_check_agi_freecount(cur, agi);
@@ -1016,12 +1025,14 @@ alloc_inode:
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -1);
+ xfs_perag_put(pag);
*inop = ino;
return 0;
error1:
xfs_btree_del_cursor(tcur, XFS_BTREE_ERROR);
error0:
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
+ xfs_perag_put(pag);
return error;
}
@@ -1052,6 +1063,7 @@ xfs_difree(
xfs_mount_t *mp; /* mount structure for filesystem */
int off; /* offset of inode in inode chunk */
xfs_inobt_rec_incore_t rec; /* btree record */
+ struct xfs_perag *pag;
mp = tp->t_mountp;
@@ -1158,7 +1170,9 @@ xfs_difree(
be32_add_cpu(&agi->agi_freecount, -(ilen - 1));
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_COUNT | XFS_AGI_FREECOUNT);
down_read(&mp->m_peraglock);
- mp->m_perag[agno].pagi_freecount -= ilen - 1;
+ pag = xfs_perag_get(mp, agno);
+ pag->pagi_freecount -= ilen - 1;
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, -ilen);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -(ilen - 1));
@@ -1189,7 +1203,9 @@ xfs_difree(
be32_add_cpu(&agi->agi_freecount, 1);
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
down_read(&mp->m_peraglock);
- mp->m_perag[agno].pagi_freecount++;
+ pag = xfs_perag_get(mp, agno);
+ pag->pagi_freecount++;
+ xfs_perag_put(pag);
up_read(&mp->m_peraglock);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
}
@@ -1379,7 +1395,6 @@ xfs_imap(
XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks));
return XFS_ERROR(EINVAL);
}
-
return 0;
}
@@ -1523,8 +1538,7 @@ xfs_ialloc_read_agi(
return error;
agi = XFS_BUF_TO_AGI(*bpp);
- pag = &mp->m_perag[agno];
-
+ pag = xfs_perag_get(mp, agno);
if (!pag->pagi_init) {
pag->pagi_freecount = be32_to_cpu(agi->agi_freecount);
pag->pagi_count = be32_to_cpu(agi->agi_count);
@@ -1537,6 +1551,7 @@ xfs_ialloc_read_agi(
*/
ASSERT(pag->pagi_freecount == be32_to_cpu(agi->agi_freecount) ||
XFS_FORCED_SHUTDOWN(mp));
+ xfs_perag_put(pag);
return 0;
}
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 7baf7c0..0a9069b 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2731,7 +2731,7 @@ xfs_iflush_cluster(
ilist_size = inodes_per_cluster * sizeof(xfs_inode_t *);
ilist = kmem_alloc(ilist_size, KM_MAYFAIL|KM_NOFS);
if (!ilist)
- return 0;
+ goto out_put;
mask = ~(((XFS_INODE_CLUSTER_SIZE(mp) >> mp->m_sb.sb_inodelog)) - 1);
first_index = XFS_INO_TO_AGINO(mp, ip->i_ino) & mask;
@@ -2800,6 +2800,8 @@ xfs_iflush_cluster(
out_free:
read_unlock(&pag->pag_ici_lock);
kmem_free(ilist);
+out_put:
+ xfs_perag_put(pag);
return 0;
@@ -2843,6 +2845,7 @@ cluster_corrupt_out:
*/
xfs_iflush_abort(iq);
kmem_free(ilist);
+ xfs_perag_put(pag);
return XFS_ERROR(EFSCORRUPTED);
}
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 66a888a..3727104 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -436,18 +436,20 @@ xfs_initialize_perag(
}
/* This ag is preferred for inodes */
- pag = &mp->m_perag[index];
+ pag = xfs_perag_get(mp, index);
pag->pagi_inodeok = 1;
if (index < max_metadata)
pag->pagf_metadata = 1;
xfs_initialize_perag_icache(pag);
+ xfs_perag_put(pag);
}
} else {
/* Setup default behavior for smaller filesystems */
for (index = 0; index < agcount; index++) {
- pag = &mp->m_perag[index];
+ pag = xfs_perag_get(mp, index);
pag->pagi_inodeok = 1;
xfs_initialize_perag_icache(pag);
+ xfs_perag_put(pag);
}
}
return index;
@@ -729,12 +731,13 @@ xfs_initialize_perag_data(xfs_mount_t *mp, xfs_agnumber_t agcount)
error = xfs_ialloc_pagi_init(mp, NULL, index);
if (error)
return error;
- pag = &mp->m_perag[index];
+ pag = xfs_perag_get(mp, index);
ifree += pag->pagi_freecount;
ialloc += pag->pagi_count;
bfree += pag->pagf_freeblks;
bfreelst += pag->pagf_flcount;
btree += pag->pagf_btreeblks;
+ xfs_perag_put(pag);
}
/*
* Overwrite incore superblock counters with just-read data
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] XFS: Replace per-ag array with a radix tree
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
` (3 preceding siblings ...)
2009-12-14 23:11 ` [PATCH 4/6] XFS: convert remaining direct references to m_perag Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-14 23:11 ` [PATCH 6/6] XFS: Reference count per-ag structures Dave Chinner
2009-12-15 5:14 ` [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
The use of an array for the per-ag structures requires reallocation of the
array when growing the filesystem. This requires locking access to the array to
avoid use after free situations, and the locking is difficult to get right. To
avoid needing to reallocate an array, change the per-ag structures to an
allocated object per ag and index them using a tree structure.
The AGs are always densely indexed (hence the use of an array), but the number
supported is 2^32 and lookups tend to be random and hence indexing needs to
scale. A simple choice is a radix tree - it works well with this sort of index.
This change also removes another large contiguous allocation from the
mount/growfs path in XFS.
The growing process now needs to change to only initialise the new AGs required
for the extra space, and as such only needs to exclusively lock the tree for
inserts. The rest of the code only needs to lock the tree while doing lookups,
and hence this will remove all the deadlocks that currently occur on the
m_perag_lock as it is now an innermost lock. The lock is also changed to a
spinlock from a read/write lock as the hold time is now extremely short.
To complete the picture, the per-ag structures will need to be reference
counted to ensure that we don't free/modify them while they are still in use.
This will be done in subsequent patch.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_alloc.c | 8 ------
fs/xfs/xfs_bmap.c | 7 +----
fs/xfs/xfs_filestream.c | 13 +++------
fs/xfs/xfs_fsops.c | 42 +++++++++++++++---------------
fs/xfs/xfs_ialloc.c | 25 +----------------
fs/xfs/xfs_itable.c | 4 ---
fs/xfs/xfs_mount.c | 65 +++++++++++++++++++++++++++++++++++-----------
fs/xfs/xfs_mount.h | 14 +++++++---
8 files changed, 87 insertions(+), 91 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index 34873ac..d1349b1 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -2419,7 +2419,6 @@ xfs_alloc_vextent(
* These three force us into a single a.g.
*/
args->agno = XFS_FSB_TO_AGNO(mp, args->fsbno);
- down_read(&mp->m_peraglock);
args->pag = xfs_perag_get(mp, args->agno);
args->minleft = 0;
error = xfs_alloc_fix_freelist(args, 0);
@@ -2429,14 +2428,12 @@ xfs_alloc_vextent(
goto error0;
}
if (!args->agbp) {
- up_read(&mp->m_peraglock);
TRACE_ALLOC("noagbp", args);
break;
}
args->agbno = XFS_FSB_TO_AGBNO(mp, args->fsbno);
if ((error = xfs_alloc_ag_vextent(args)))
goto error0;
- up_read(&mp->m_peraglock);
break;
case XFS_ALLOCTYPE_START_BNO:
/*
@@ -2488,7 +2485,6 @@ xfs_alloc_vextent(
* Loop over allocation groups twice; first time with
* trylock set, second time without.
*/
- down_read(&mp->m_peraglock);
for (;;) {
args->pag = xfs_perag_get(mp, args->agno);
if (no_min) args->minleft = 0;
@@ -2549,7 +2545,6 @@ xfs_alloc_vextent(
}
xfs_perag_put(args->pag);
}
- up_read(&mp->m_peraglock);
if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
if (args->agno == sagno)
mp->m_agfrotor = (mp->m_agfrotor + 1) %
@@ -2579,7 +2574,6 @@ xfs_alloc_vextent(
return 0;
error0:
xfs_perag_put(args->pag);
- up_read(&mp->m_peraglock);
return error;
}
@@ -2604,7 +2598,6 @@ xfs_free_extent(
args.agno = XFS_FSB_TO_AGNO(args.mp, bno);
ASSERT(args.agno < args.mp->m_sb.sb_agcount);
args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);
- down_read(&args.mp->m_peraglock);
args.pag = xfs_perag_get(args.mp, args.agno);
if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))
goto error0;
@@ -2616,7 +2609,6 @@ xfs_free_extent(
error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);
error0:
xfs_perag_put(args.pag);
- up_read(&args.mp->m_peraglock);
return error;
}
diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
index 719fa11..4847fb0 100644
--- a/fs/xfs/xfs_bmap.c
+++ b/fs/xfs/xfs_bmap.c
@@ -2780,14 +2780,12 @@ xfs_bmap_btalloc(
if (startag == NULLAGNUMBER)
startag = ag = 0;
notinit = 0;
- down_read(&mp->m_peraglock);
pag = xfs_perag_get(mp, ag);
while (blen < ap->alen) {
if (!pag->pagf_init &&
(error = xfs_alloc_pagf_init(mp, args.tp,
ag, XFS_ALLOC_FLAG_TRYLOCK))) {
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
return error;
}
/*
@@ -2820,10 +2818,8 @@ xfs_bmap_btalloc(
error = xfs_filestream_new_ag(ap, &ag);
xfs_perag_put(pag);
- if (error) {
- up_read(&mp->m_peraglock);
+ if (error)
return error;
- }
/* loop again to set 'blen'*/
startag = NULLAGNUMBER;
@@ -2839,7 +2835,6 @@ xfs_bmap_btalloc(
pag = xfs_perag_get(mp, ag);
}
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
/*
* Since the above loop did a BUF_TRYLOCK, it is
* possible that there is space for this request.
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 08c27ba..bfe9777 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -252,8 +252,7 @@ next_ag:
/*
* Set the allocation group number for a file or a directory, updating inode
- * references and per-AG references as appropriate. Must be called with the
- * m_peraglock held in read mode.
+ * references and per-AG references as appropriate.
*/
static int
_xfs_filestream_update_ag(
@@ -460,10 +459,10 @@ xfs_filestream_unmount(
}
/*
- * If the mount point's m_perag array is going to be reallocated, all
+ * If the mount point's m_perag tree is going to be modified, all
* outstanding cache entries must be flushed to avoid accessing reference count
* addresses that have been freed. The call to xfs_filestream_flush() must be
- * made inside the block that holds the m_peraglock in write mode to do the
+ * made inside the block that holds the m_perag_lock in write mode to do the
* reallocation.
*/
void
@@ -535,7 +534,6 @@ xfs_filestream_associate(
mp = pip->i_mount;
cache = mp->m_filestream;
- down_read(&mp->m_peraglock);
/*
* We have a problem, Houston.
@@ -552,10 +550,8 @@ xfs_filestream_associate(
*
* So, if we can't get the iolock without sleeping then just give up
*/
- if (!xfs_ilock_nowait(pip, XFS_IOLOCK_EXCL)) {
- up_read(&mp->m_peraglock);
+ if (!xfs_ilock_nowait(pip, XFS_IOLOCK_EXCL))
return 1;
- }
/* If the parent directory is already in the cache, use its AG. */
item = xfs_mru_cache_lookup(cache, pip->i_ino);
@@ -610,7 +606,6 @@ exit_did_pick:
exit:
xfs_iunlock(pip, XFS_IOLOCK_EXCL);
- up_read(&mp->m_peraglock);
return -err;
}
diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 36079aa..c0fcad8 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -166,27 +166,14 @@ xfs_growfs_data_private(
}
new = nb - mp->m_sb.sb_dblocks;
oagcount = mp->m_sb.sb_agcount;
- if (nagcount > oagcount) {
- void *new_perag, *old_perag;
-
- xfs_filestream_flush(mp);
-
- new_perag = kmem_zalloc(sizeof(xfs_perag_t) * nagcount,
- KM_MAYFAIL);
- if (!new_perag)
- return XFS_ERROR(ENOMEM);
-
- down_write(&mp->m_peraglock);
- memcpy(new_perag, mp->m_perag, sizeof(xfs_perag_t) * oagcount);
- old_perag = mp->m_perag;
- mp->m_perag = new_perag;
-
- mp->m_flags |= XFS_MOUNT_32BITINODES;
- nagimax = xfs_initialize_perag(mp, nagcount);
- up_write(&mp->m_peraglock);
- kmem_free(old_perag);
+ /* allocate the new per-ag structures */
+ if (nagcount > oagcount) {
+ error = xfs_initialize_perag(mp, nagcount, &nagimax);
+ if (error)
+ return error;
}
+
tp = xfs_trans_alloc(mp, XFS_TRANS_GROWFS);
tp->t_flags |= XFS_TRANS_RESERVE;
if ((error = xfs_trans_reserve(tp, XFS_GROWFS_SPACE_RES(mp),
@@ -195,6 +182,11 @@ xfs_growfs_data_private(
return error;
}
+ /*
+ * Write new AG headers to disk. Non-transactional, but written
+ * synchronously so they are completed prior to the growfs transaction
+ * being logged.
+ */
nfree = 0;
for (agno = nagcount - 1; agno >= oagcount; agno--, new -= agsize) {
/*
@@ -357,6 +349,12 @@ xfs_growfs_data_private(
goto error0;
}
}
+
+ /*
+ * Update changed superblock fields transactionally. These are not
+ * seen by the rest of the world until the transaction commit applies
+ * them atomically to the superblock.
+ */
if (nagcount > oagcount)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_AGCOUNT, nagcount - oagcount);
if (nb > mp->m_sb.sb_dblocks)
@@ -367,9 +365,9 @@ xfs_growfs_data_private(
if (dpct)
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IMAXPCT, dpct);
error = xfs_trans_commit(tp, 0);
- if (error) {
+ if (error)
return error;
- }
+
/* New allocation groups fully initialized, so update mount struct */
if (nagimax)
mp->m_maxagi = nagimax;
@@ -379,6 +377,8 @@ xfs_growfs_data_private(
mp->m_maxicount = icount << mp->m_sb.sb_inopblog;
} else
mp->m_maxicount = 0;
+
+ /* update secondary superblocks. */
for (agno = 1; agno < nagcount; agno++) {
error = xfs_read_buf(mp, mp->m_ddev_targp,
XFS_AGB_TO_DADDR(mp, agno, XFS_SB_BLOCK(mp)),
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 884ee13..52c9d00 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -383,11 +383,9 @@ xfs_ialloc_ag_alloc(
newino = XFS_OFFBNO_TO_AGINO(args.mp, args.agbno, 0);
be32_add_cpu(&agi->agi_count, newlen);
be32_add_cpu(&agi->agi_freecount, newlen);
- down_read(&args.mp->m_peraglock);
pag = xfs_perag_get(args.mp, agno);
pag->pagi_freecount += newlen;
xfs_perag_put(pag);
- up_read(&args.mp->m_peraglock);
agi->agi_newino = cpu_to_be32(newino);
/*
@@ -489,7 +487,6 @@ xfs_ialloc_ag_select(
*/
agno = pagno;
flags = XFS_ALLOC_FLAG_TRYLOCK;
- down_read(&mp->m_peraglock);
for (;;) {
pag = xfs_perag_get(mp, agno);
if (!pag->pagi_init) {
@@ -531,7 +528,6 @@ xfs_ialloc_ag_select(
goto nextag;
}
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
return agbp;
}
}
@@ -544,18 +540,14 @@ nextag:
* No point in iterating over the rest, if we're shutting
* down.
*/
- if (XFS_FORCED_SHUTDOWN(mp)) {
- up_read(&mp->m_peraglock);
+ if (XFS_FORCED_SHUTDOWN(mp))
return NULL;
- }
agno++;
if (agno >= agcount)
agno = 0;
if (agno == pagno) {
- if (flags == 0) {
- up_read(&mp->m_peraglock);
+ if (flags == 0)
return NULL;
- }
flags = 0;
}
}
@@ -777,16 +769,13 @@ nextag:
*inop = NULLFSINO;
return noroom ? ENOSPC : 0;
}
- down_read(&mp->m_peraglock);
pag = xfs_perag_get(mp, tagno);
if (pag->pagi_inodeok == 0) {
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
goto nextag;
}
error = xfs_ialloc_read_agi(mp, tp, tagno, &agbp);
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
if (error)
goto nextag;
agi = XFS_BUF_TO_AGI(agbp);
@@ -1015,9 +1004,7 @@ alloc_inode:
goto error0;
be32_add_cpu(&agi->agi_freecount, -1);
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
- down_read(&mp->m_peraglock);
pag->pagi_freecount--;
- up_read(&mp->m_peraglock);
error = xfs_check_agi_freecount(cur, agi);
if (error)
@@ -1100,9 +1087,7 @@ xfs_difree(
/*
* Get the allocation group header.
*/
- down_read(&mp->m_peraglock);
error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
- up_read(&mp->m_peraglock);
if (error) {
cmn_err(CE_WARN,
"xfs_difree: xfs_ialloc_read_agi() returned an error %d on %s. Returning error.",
@@ -1169,11 +1154,9 @@ xfs_difree(
be32_add_cpu(&agi->agi_count, -ilen);
be32_add_cpu(&agi->agi_freecount, -(ilen - 1));
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_COUNT | XFS_AGI_FREECOUNT);
- down_read(&mp->m_peraglock);
pag = xfs_perag_get(mp, agno);
pag->pagi_freecount -= ilen - 1;
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_ICOUNT, -ilen);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, -(ilen - 1));
@@ -1202,11 +1185,9 @@ xfs_difree(
*/
be32_add_cpu(&agi->agi_freecount, 1);
xfs_ialloc_log_agi(tp, agbp, XFS_AGI_FREECOUNT);
- down_read(&mp->m_peraglock);
pag = xfs_perag_get(mp, agno);
pag->pagi_freecount++;
xfs_perag_put(pag);
- up_read(&mp->m_peraglock);
xfs_trans_mod_sb(tp, XFS_TRANS_SB_IFREE, 1);
}
@@ -1328,9 +1309,7 @@ xfs_imap(
xfs_buf_t *agbp; /* agi buffer */
int i; /* temp state */
- down_read(&mp->m_peraglock);
error = xfs_ialloc_read_agi(mp, tp, agno, &agbp);
- up_read(&mp->m_peraglock);
if (error) {
xfs_fs_cmn_err(CE_ALERT, mp, "xfs_imap: "
"xfs_ialloc_read_agi() returned "
diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
index 62efab2..940307a 100644
--- a/fs/xfs/xfs_itable.c
+++ b/fs/xfs/xfs_itable.c
@@ -420,9 +420,7 @@ xfs_bulkstat(
while (XFS_BULKSTAT_UBLEFT(ubleft) && agno < mp->m_sb.sb_agcount) {
cond_resched();
bp = NULL;
- down_read(&mp->m_peraglock);
error = xfs_ialloc_read_agi(mp, NULL, agno, &agbp);
- up_read(&mp->m_peraglock);
if (error) {
/*
* Skip this allocation group and go to the next one.
@@ -849,9 +847,7 @@ xfs_inumbers(
agbp = NULL;
while (left > 0 && agno < mp->m_sb.sb_agcount) {
if (agbp == NULL) {
- down_read(&mp->m_peraglock);
error = xfs_ialloc_read_agi(mp, NULL, agno, &agbp);
- up_read(&mp->m_peraglock);
if (error) {
/*
* If we can't read the AGI of this ag,
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3727104..1ab9495 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -207,13 +207,16 @@ STATIC void
xfs_free_perag(
xfs_mount_t *mp)
{
- if (mp->m_perag) {
- int agno;
+ xfs_agnumber_t agno;
+ struct xfs_perag *pag;
- for (agno = 0; agno < mp->m_maxagi; agno++)
- if (mp->m_perag[agno].pagb_list)
- kmem_free(mp->m_perag[agno].pagb_list);
- kmem_free(mp->m_perag);
+ for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
+ spin_lock(&mp->m_perag_lock);
+ pag = radix_tree_delete(&mp->m_perag_tree, agno);
+ spin_unlock(&mp->m_perag_lock);
+ ASSERT(pag);
+ kmem_free(pag->pagb_list);
+ kmem_free(pag);
}
}
@@ -387,10 +390,11 @@ xfs_initialize_perag_icache(
}
}
-xfs_agnumber_t
+int
xfs_initialize_perag(
xfs_mount_t *mp,
- xfs_agnumber_t agcount)
+ xfs_agnumber_t agcount,
+ xfs_agnumber_t *maxagi)
{
xfs_agnumber_t index, max_metadata;
xfs_perag_t *pag;
@@ -403,6 +407,33 @@ xfs_initialize_perag(
agino = XFS_OFFBNO_TO_AGINO(mp, sbp->sb_agblocks - 1, 0);
ino = XFS_AGINO_TO_INO(mp, agcount - 1, agino);
+ /*
+ * Walk the current per-ag tree so we don't try to initialise AGs
+ * that already exist (growfs case). Allocate and insert all the
+ * AGs we don't find ready for initialisation.
+ */
+ for (index = 0; index < agcount; index++) {
+ pag = xfs_perag_get(mp, index);
+ if (pag) {
+ xfs_perag_put(pag);
+ continue;
+ }
+ pag = kmem_zalloc(sizeof(*pag), KM_MAYFAIL);
+ if (!pag)
+ return -ENOMEM;
+ if (radix_tree_preload(GFP_NOFS))
+ return -ENOMEM;
+ spin_lock(&mp->m_perag_lock);
+ if (radix_tree_insert(&mp->m_perag_tree, index, pag)) {
+ BUG();
+ spin_unlock(&mp->m_perag_lock);
+ kmem_free(pag);
+ return -EEXIST;
+ }
+ spin_unlock(&mp->m_perag_lock);
+ radix_tree_preload_end();
+ }
+
/* Clear the mount flag if no inode can overflow 32 bits
* on this filesystem, or if specifically requested..
*/
@@ -452,7 +483,9 @@ xfs_initialize_perag(
xfs_perag_put(pag);
}
}
- return index;
+ if (maxagi)
+ *maxagi = index;
+ return 0;
}
void
@@ -1153,13 +1186,13 @@ xfs_mountfs(
/*
* Allocate and initialize the per-ag data.
*/
- init_rwsem(&mp->m_peraglock);
- mp->m_perag = kmem_zalloc(sbp->sb_agcount * sizeof(xfs_perag_t),
- KM_MAYFAIL);
- if (!mp->m_perag)
+ spin_lock_init(&mp->m_perag_lock);
+ INIT_RADIX_TREE(&mp->m_perag_tree, GFP_NOFS);
+ error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
+ if (error) {
+ cmn_err(CE_WARN, "XFS: Failed per-ag init: %d", error);
goto out_remove_uuid;
-
- mp->m_maxagi = xfs_initialize_perag(mp, sbp->sb_agcount);
+ }
if (!sbp->sb_logblocks) {
cmn_err(CE_WARN, "XFS: no log defined");
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index f8a68a2..cfa7a5d 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -207,8 +207,8 @@ typedef struct xfs_mount {
uint m_ag_maxlevels; /* XFS_AG_MAXLEVELS */
uint m_bm_maxlevels[2]; /* XFS_BM_MAXLEVELS */
uint m_in_maxlevels; /* max inobt btree levels. */
- struct xfs_perag *m_perag; /* per-ag accounting info */
- struct rw_semaphore m_peraglock; /* lock for m_perag (pointer) */
+ struct radix_tree_root m_perag_tree; /* per-ag accounting info */
+ spinlock_t m_perag_lock; /* lock for m_perag_tree */
struct mutex m_growlock; /* growfs mutex */
int m_fixedfsid[2]; /* unchanged for life of FS */
uint m_dmevmask; /* DMI events for this FS */
@@ -389,7 +389,12 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
static inline struct xfs_perag *
xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
{
- return &mp->m_perag[agno];
+ struct xfs_perag *pag;
+
+ spin_lock(&mp->m_perag_lock);
+ pag = radix_tree_lookup(&mp->m_perag_tree, agno);
+ spin_unlock(&mp->m_perag_lock);
+ return pag;
}
static inline void
@@ -450,7 +455,8 @@ extern struct xfs_dmops xfs_dmcore_xfs;
#endif /* __KERNEL__ */
extern void xfs_mod_sb(struct xfs_trans *, __int64_t);
-extern xfs_agnumber_t xfs_initialize_perag(struct xfs_mount *, xfs_agnumber_t);
+extern int xfs_initialize_perag(struct xfs_mount *, xfs_agnumber_t,
+ xfs_agnumber_t *);
extern void xfs_sb_from_disk(struct xfs_sb *, struct xfs_dsb *);
extern void xfs_sb_to_disk(struct xfs_dsb *, struct xfs_sb *, __int64_t);
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/6] XFS: Reference count per-ag structures
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
` (4 preceding siblings ...)
2009-12-14 23:11 ` [PATCH 5/6] XFS: Replace per-ag array with a radix tree Dave Chinner
@ 2009-12-14 23:11 ` Dave Chinner
2009-12-15 5:14 ` [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-14 23:11 UTC (permalink / raw)
To: xfs
Reference count the per-ag structures to ensure that we keep get/put
pairs balanced. Assert that the reference counts are zero at unmount
time to catch leaks. In future, reference counts will enable us to
safely remove perag structures by allowing us to detect when they
are no longer in use.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_ag.h | 4 ++--
fs/xfs/xfs_mount.c | 1 +
fs/xfs/xfs_mount.h | 11 +++++++++--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
index a5d54bf..b384a3c 100644
--- a/fs/xfs/xfs_ag.h
+++ b/fs/xfs/xfs_ag.h
@@ -182,8 +182,8 @@ typedef struct xfs_perag_busy {
#define XFS_PAGB_NUM_SLOTS 128
#endif
-typedef struct xfs_perag
-{
+typedef struct xfs_perag {
+ atomic_t pag_ref; /* perag reference count */
char pagf_init; /* this agf's entry is initialized */
char pagi_init; /* this agi's entry is initialized */
char pagf_metadata; /* the agf is preferred to be metadata */
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 1ab9495..0203a6a 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -213,6 +213,7 @@ xfs_free_perag(
for (agno = 0; agno < mp->m_sb.sb_agcount; agno++) {
spin_lock(&mp->m_perag_lock);
pag = radix_tree_delete(&mp->m_perag_tree, agno);
+ ASSERT(atomic_read(&pag->pag_ref) == 0);
spin_unlock(&mp->m_perag_lock);
ASSERT(pag);
kmem_free(pag->pagb_list);
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index cfa7a5d..16b2212 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -384,7 +384,7 @@ xfs_daddr_to_agbno(struct xfs_mount *mp, xfs_daddr_t d)
}
/*
- * perag get/put wrappers for eventual ref counting
+ * perag get/put wrappers for ref counting
*/
static inline struct xfs_perag *
xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
@@ -393,6 +393,12 @@ xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
spin_lock(&mp->m_perag_lock);
pag = radix_tree_lookup(&mp->m_perag_tree, agno);
+ if (pag) {
+ ASSERT(atomic_read(&pag->pag_ref) >= 0);
+ /* catch leaks in the positive direction during testing */
+ ASSERT(atomic_read(&pag->pag_ref) < 1000);
+ atomic_inc(&pag->pag_ref);
+ }
spin_unlock(&mp->m_perag_lock);
return pag;
}
@@ -400,7 +406,8 @@ xfs_perag_get(struct xfs_mount *mp, xfs_agnumber_t agno)
static inline void
xfs_perag_put(struct xfs_perag *pag)
{
- /* nothing to see here, move along */
+ ASSERT(atomic_read(&pag->pag_ref) > 0);
+ atomic_dec(&pag->pag_ref);
}
/*
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2
2009-12-14 23:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V2 Dave Chinner
` (5 preceding siblings ...)
2009-12-14 23:11 ` [PATCH 6/6] XFS: Reference count per-ag structures Dave Chinner
@ 2009-12-15 5:14 ` Dave Chinner
6 siblings, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-15 5:14 UTC (permalink / raw)
To: xfs
On Tue, Dec 15, 2009 at 10:11:04AM +1100, Dave Chinner wrote:
> The use of an array for the xfs_perag structures results in growfs having
> to realocate the array. This requires exclusion to prevent use-after-free
> situations. The current locking is prone to deadlocks when growing under
> load, and the xfssyncd currently has no protection against the array being
> reallocated that can lead to panics.
>
> This series abstracts the per-ag structure access and then removes the array
> to replace it with individual xfs_perag structures indexed by a radix tree.
> The only locking required is for the radix tree, hence the deadlocks go away
> as the tree lock is always the innermost lock. The use after frees go away
> as well as growfs does not need to reallocate structures fo pre-existing
> allocation groups - it only needs to allocate the structures for the new
> AGs and insert them into the tree.
>
> This series also adds reference counting to the xfs-perag structure to
> ensure that we balance get/put accesses to the structures and provide the
> infrastructure to determine if the structure is in use or not.
>
> Version 2: Fix review comments from Christoph.
This series needs updating after the trace patches were merged. I'll
post it again soon...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code
2009-12-15 6:11 [PATCH 0/6] XFS: Fix growfs deadlocks and per-AG use after free V3 Dave Chinner
@ 2009-12-15 6:11 ` Dave Chinner
2009-12-20 4:24 ` Dave Chinner
2009-12-22 14:18 ` Alex Elder
0 siblings, 2 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-15 6:11 UTC (permalink / raw)
To: xfs
Start abstracting the perag references so that the indexing of the
structures is not directly coded into all the places that uses the
perag structures. This will allow us to separate the use of the
perag structure and the way it is indexed and hence avoid the known
deadlocks related to growing a busy filesystem.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_alloc.c | 81 +++++++++++++++++++++++++--------------------
fs/xfs/xfs_alloc_btree.c | 9 ++++-
2 files changed, 52 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index a1c65fc..3cb533c 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -1662,11 +1662,13 @@ xfs_free_ag_extent(
xfs_agf_t *agf;
xfs_perag_t *pag; /* per allocation group data */
+ pag = xfs_perag_get(mp, agno);
+ pag->pagf_freeblks += len;
+ xfs_perag_put(pag);
+
agf = XFS_BUF_TO_AGF(agbp);
- pag = &mp->m_perag[agno];
be32_add_cpu(&agf->agf_freeblks, len);
xfs_trans_agblocks_delta(tp, len);
- pag->pagf_freeblks += len;
XFS_WANT_CORRUPTED_GOTO(
be32_to_cpu(agf->agf_freeblks) <=
be32_to_cpu(agf->agf_length),
@@ -1969,7 +1971,8 @@ xfs_alloc_get_freelist(
xfs_trans_brelse(tp, agflbp);
if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
agf->agf_flfirst = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, -1);
xfs_trans_agflist_delta(tp, -1);
pag->pagf_flcount--;
@@ -2078,7 +2081,8 @@ xfs_alloc_put_freelist(
be32_add_cpu(&agf->agf_fllast, 1);
if (be32_to_cpu(agf->agf_fllast) == XFS_AGFL_SIZE(mp))
agf->agf_fllast = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, 1);
xfs_trans_agflist_delta(tp, 1);
pag->pagf_flcount++;
@@ -2089,6 +2093,7 @@ xfs_alloc_put_freelist(
pag->pagf_btreeblks--;
logflags |= XFS_AGF_BTREEBLKS;
}
+ xfs_perag_put(pag);
xfs_alloc_log_agf(tp, agbp, logflags);
@@ -2152,7 +2157,6 @@ xfs_read_agf(
xfs_trans_brelse(tp, *bpp);
return XFS_ERROR(EFSCORRUPTED);
}
-
XFS_BUF_SET_VTYPE_REF(*bpp, B_FS_AGF, XFS_AGF_REF);
return 0;
}
@@ -2184,7 +2188,7 @@ xfs_alloc_read_agf(
ASSERT(!XFS_BUF_GETERROR(*bpp));
agf = XFS_BUF_TO_AGF(*bpp);
- pag = &mp->m_perag[agno];
+ pag = xfs_perag_get(mp, agno);
if (!pag->pagf_init) {
pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
pag->pagf_btreeblks = be32_to_cpu(agf->agf_btreeblks);
@@ -2211,6 +2215,7 @@ xfs_alloc_read_agf(
be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNTi]));
}
#endif
+ xfs_perag_put(pag);
return 0;
}
@@ -2271,7 +2276,7 @@ xfs_alloc_vextent(
*/
args->agno = XFS_FSB_TO_AGNO(mp, args->fsbno);
down_read(&mp->m_peraglock);
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
args->minleft = 0;
error = xfs_alloc_fix_freelist(args, 0);
args->minleft = minleft;
@@ -2341,7 +2346,7 @@ xfs_alloc_vextent(
*/
down_read(&mp->m_peraglock);
for (;;) {
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
if (no_min) args->minleft = 0;
error = xfs_alloc_fix_freelist(args, flags);
args->minleft = minleft;
@@ -2400,6 +2405,7 @@ xfs_alloc_vextent(
}
}
}
+ xfs_perag_put(args->pag);
}
up_read(&mp->m_peraglock);
if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
@@ -2427,8 +2433,10 @@ xfs_alloc_vextent(
args->len);
#endif
}
+ xfs_perag_put(args->pag);
return 0;
error0:
+ xfs_perag_put(args->pag);
up_read(&mp->m_peraglock);
return error;
}
@@ -2455,7 +2463,7 @@ xfs_free_extent(
ASSERT(args.agno < args.mp->m_sb.sb_agcount);
args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);
down_read(&args.mp->m_peraglock);
- args.pag = &args.mp->m_perag[args.agno];
+ args.pag = xfs_perag_get(args.mp, args.agno);
if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))
goto error0;
#ifdef DEBUG
@@ -2465,6 +2473,7 @@ xfs_free_extent(
#endif
error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);
error0:
+ xfs_perag_put(args.pag);
up_read(&args.mp->m_peraglock);
return error;
}
@@ -2486,15 +2495,15 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
xfs_perag_busy_t *bsy;
+ struct xfs_perag *pag;
int n;
- mp = tp->t_mountp;
- spin_lock(&mp->m_perag[agno].pagb_lock);
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
/* search pagb_list for an open slot */
- for (bsy = mp->m_perag[agno].pagb_list, n = 0;
+ for (bsy = pag->pagb_list, n = 0;
n < XFS_PAGB_NUM_SLOTS;
bsy++, n++) {
if (bsy->busy_tp == NULL) {
@@ -2502,11 +2511,11 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
}
}
- trace_xfs_alloc_busy(mp, agno, bno, len, n);
+ trace_xfs_alloc_busy(tp->t_mountp, agno, bno, len, n);
if (n < XFS_PAGB_NUM_SLOTS) {
- bsy = &mp->m_perag[agno].pagb_list[n];
- mp->m_perag[agno].pagb_count++;
+ bsy = &pag->pagb_list[n];
+ pag->pagb_count++;
bsy->busy_start = bno;
bsy->busy_length = len;
bsy->busy_tp = tp;
@@ -2521,7 +2530,8 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_trans_set_sync(tp);
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
void
@@ -2529,24 +2539,23 @@ xfs_alloc_clear_busy(xfs_trans_t *tp,
xfs_agnumber_t agno,
int idx)
{
- xfs_mount_t *mp;
+ struct xfs_perag *pag;
xfs_perag_busy_t *list;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- list = mp->m_perag[agno].pagb_list;
-
ASSERT(idx < XFS_PAGB_NUM_SLOTS);
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ list = pag->pagb_list;
- trace_xfs_alloc_unbusy(mp, agno, idx, list[idx].busy_tp == tp);
+ trace_xfs_alloc_unbusy(tp->t_mountp, agno, idx, list[idx].busy_tp == tp);
if (list[idx].busy_tp == tp) {
list[idx].busy_tp = NULL;
- mp->m_perag[agno].pagb_count--;
+ pag->pagb_count--;
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
@@ -2560,21 +2569,20 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
+ struct xfs_perag *pag;
xfs_perag_busy_t *bsy;
xfs_agblock_t uend, bend;
xfs_lsn_t lsn;
int cnt;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- cnt = mp->m_perag[agno].pagb_count;
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ cnt = pag->pagb_count;
uend = bno + len - 1;
/* search pagb_list for this slot, skipping open slots */
- for (bsy = mp->m_perag[agno].pagb_list; cnt; bsy++) {
+ for (bsy = pag->pagb_list; cnt; bsy++) {
/*
* (start1,length1) within (start2, length2)
@@ -2589,7 +2597,7 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
}
}
- trace_xfs_alloc_busysearch(mp, agno, bno, len, !!cnt);
+ trace_xfs_alloc_busysearch(tp->t_mountp, agno, bno, len, !!cnt);
/*
* If a block was found, force the log through the LSN of the
@@ -2597,9 +2605,10 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
*/
if (cnt) {
lsn = bsy->busy_tp->t_commit_lsn;
- spin_unlock(&mp->m_perag[agno].pagb_lock);
- xfs_log_force(mp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
+ spin_unlock(&pag->pagb_lock);
+ xfs_log_force(tp->t_mountp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
} else {
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
}
+ xfs_perag_put(pag);
}
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index adbd914..b726e10 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -61,12 +61,14 @@ xfs_allocbt_set_root(
struct xfs_agf *agf = XFS_BUF_TO_AGF(agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
int btnum = cur->bc_btnum;
+ struct xfs_perag *pag = xfs_perag_get(cur->bc_mp, seqno);
ASSERT(ptr->s != 0);
agf->agf_roots[btnum] = ptr->s;
be32_add_cpu(&agf->agf_levels[btnum], inc);
- cur->bc_mp->m_perag[seqno].pagf_levels[btnum] += inc;
+ pag->pagf_levels[btnum] += inc;
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_ROOTS | XFS_AGF_LEVELS);
}
@@ -150,6 +152,7 @@ xfs_allocbt_update_lastrec(
{
struct xfs_agf *agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
+ struct xfs_perag *pag;
__be32 len;
int numrecs;
@@ -193,7 +196,9 @@ xfs_allocbt_update_lastrec(
}
agf->agf_longest = len;
- cur->bc_mp->m_perag[seqno].pagf_longest = be32_to_cpu(len);
+ pag = xfs_perag_get(cur->bc_mp, seqno);
+ pag->pagf_longest = be32_to_cpu(len);
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, cur->bc_private.a.agbp, XFS_AGF_LONGEST);
}
--
1.6.5
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code
2009-12-15 6:11 ` [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code Dave Chinner
@ 2009-12-20 4:24 ` Dave Chinner
2009-12-22 14:18 ` Alex Elder
1 sibling, 0 replies; 14+ messages in thread
From: Dave Chinner @ 2009-12-20 4:24 UTC (permalink / raw)
To: xfs
On Tue, Dec 15, 2009 at 05:11:13PM +1100, Dave Chinner wrote:
> Start abstracting the perag references so that the indexing of the
> structures is not directly coded into all the places that uses the
> perag structures. This will allow us to separate the use of the
> perag structure and the way it is indexed and hence avoid the known
> deadlocks related to growing a busy filesystem.
>
> Signed-off-by: Dave Chinner <david@fromorbit.com>
I screwed up the rebase so this patch is missing a xfs_perag_put()
call in xfs_alloc_get_freelist(), so will assert fail on unmount in
test 042. Corrected patch below.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
XFS: Don't directly reference m_perag in allocation code
Start abstracting the perag references so that the indexing of the
structures is not directly coded into all the places that uses the
perag structures. This will allow us to separate the use of the
perag structure and the way it is indexed and hence avoid the known
deadlocks related to growing a busy filesystem.
Signed-off-by: Dave Chinner <david@fromorbit.com>
---
fs/xfs/xfs_alloc.c | 82 ++++++++++++++++++++++++++--------------------
fs/xfs/xfs_alloc_btree.c | 9 ++++-
2 files changed, 53 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
index a1c65fc..d39ee2d 100644
--- a/fs/xfs/xfs_alloc.c
+++ b/fs/xfs/xfs_alloc.c
@@ -1662,11 +1662,13 @@ xfs_free_ag_extent(
xfs_agf_t *agf;
xfs_perag_t *pag; /* per allocation group data */
+ pag = xfs_perag_get(mp, agno);
+ pag->pagf_freeblks += len;
+ xfs_perag_put(pag);
+
agf = XFS_BUF_TO_AGF(agbp);
- pag = &mp->m_perag[agno];
be32_add_cpu(&agf->agf_freeblks, len);
xfs_trans_agblocks_delta(tp, len);
- pag->pagf_freeblks += len;
XFS_WANT_CORRUPTED_GOTO(
be32_to_cpu(agf->agf_freeblks) <=
be32_to_cpu(agf->agf_length),
@@ -1969,10 +1971,12 @@ xfs_alloc_get_freelist(
xfs_trans_brelse(tp, agflbp);
if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
agf->agf_flfirst = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, -1);
xfs_trans_agflist_delta(tp, -1);
pag->pagf_flcount--;
+ xfs_perag_put(pag);
logflags = XFS_AGF_FLFIRST | XFS_AGF_FLCOUNT;
if (btreeblk) {
@@ -2078,7 +2082,8 @@ xfs_alloc_put_freelist(
be32_add_cpu(&agf->agf_fllast, 1);
if (be32_to_cpu(agf->agf_fllast) == XFS_AGFL_SIZE(mp))
agf->agf_fllast = 0;
- pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
+
+ pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
be32_add_cpu(&agf->agf_flcount, 1);
xfs_trans_agflist_delta(tp, 1);
pag->pagf_flcount++;
@@ -2089,6 +2094,7 @@ xfs_alloc_put_freelist(
pag->pagf_btreeblks--;
logflags |= XFS_AGF_BTREEBLKS;
}
+ xfs_perag_put(pag);
xfs_alloc_log_agf(tp, agbp, logflags);
@@ -2152,7 +2158,6 @@ xfs_read_agf(
xfs_trans_brelse(tp, *bpp);
return XFS_ERROR(EFSCORRUPTED);
}
-
XFS_BUF_SET_VTYPE_REF(*bpp, B_FS_AGF, XFS_AGF_REF);
return 0;
}
@@ -2184,7 +2189,7 @@ xfs_alloc_read_agf(
ASSERT(!XFS_BUF_GETERROR(*bpp));
agf = XFS_BUF_TO_AGF(*bpp);
- pag = &mp->m_perag[agno];
+ pag = xfs_perag_get(mp, agno);
if (!pag->pagf_init) {
pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
pag->pagf_btreeblks = be32_to_cpu(agf->agf_btreeblks);
@@ -2211,6 +2216,7 @@ xfs_alloc_read_agf(
be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNTi]));
}
#endif
+ xfs_perag_put(pag);
return 0;
}
@@ -2271,7 +2277,7 @@ xfs_alloc_vextent(
*/
args->agno = XFS_FSB_TO_AGNO(mp, args->fsbno);
down_read(&mp->m_peraglock);
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
args->minleft = 0;
error = xfs_alloc_fix_freelist(args, 0);
args->minleft = minleft;
@@ -2341,7 +2347,7 @@ xfs_alloc_vextent(
*/
down_read(&mp->m_peraglock);
for (;;) {
- args->pag = &mp->m_perag[args->agno];
+ args->pag = xfs_perag_get(mp, args->agno);
if (no_min) args->minleft = 0;
error = xfs_alloc_fix_freelist(args, flags);
args->minleft = minleft;
@@ -2400,6 +2406,7 @@ xfs_alloc_vextent(
}
}
}
+ xfs_perag_put(args->pag);
}
up_read(&mp->m_peraglock);
if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
@@ -2427,8 +2434,10 @@ xfs_alloc_vextent(
args->len);
#endif
}
+ xfs_perag_put(args->pag);
return 0;
error0:
+ xfs_perag_put(args->pag);
up_read(&mp->m_peraglock);
return error;
}
@@ -2455,7 +2464,7 @@ xfs_free_extent(
ASSERT(args.agno < args.mp->m_sb.sb_agcount);
args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);
down_read(&args.mp->m_peraglock);
- args.pag = &args.mp->m_perag[args.agno];
+ args.pag = xfs_perag_get(args.mp, args.agno);
if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))
goto error0;
#ifdef DEBUG
@@ -2465,6 +2474,7 @@ xfs_free_extent(
#endif
error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);
error0:
+ xfs_perag_put(args.pag);
up_read(&args.mp->m_peraglock);
return error;
}
@@ -2486,15 +2496,15 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
xfs_perag_busy_t *bsy;
+ struct xfs_perag *pag;
int n;
- mp = tp->t_mountp;
- spin_lock(&mp->m_perag[agno].pagb_lock);
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
/* search pagb_list for an open slot */
- for (bsy = mp->m_perag[agno].pagb_list, n = 0;
+ for (bsy = pag->pagb_list, n = 0;
n < XFS_PAGB_NUM_SLOTS;
bsy++, n++) {
if (bsy->busy_tp == NULL) {
@@ -2502,11 +2512,11 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
}
}
- trace_xfs_alloc_busy(mp, agno, bno, len, n);
+ trace_xfs_alloc_busy(tp->t_mountp, agno, bno, len, n);
if (n < XFS_PAGB_NUM_SLOTS) {
- bsy = &mp->m_perag[agno].pagb_list[n];
- mp->m_perag[agno].pagb_count++;
+ bsy = &pag->pagb_list[n];
+ pag->pagb_count++;
bsy->busy_start = bno;
bsy->busy_length = len;
bsy->busy_tp = tp;
@@ -2521,7 +2531,8 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
xfs_trans_set_sync(tp);
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
void
@@ -2529,24 +2540,23 @@ xfs_alloc_clear_busy(xfs_trans_t *tp,
xfs_agnumber_t agno,
int idx)
{
- xfs_mount_t *mp;
+ struct xfs_perag *pag;
xfs_perag_busy_t *list;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- list = mp->m_perag[agno].pagb_list;
-
ASSERT(idx < XFS_PAGB_NUM_SLOTS);
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ list = pag->pagb_list;
- trace_xfs_alloc_unbusy(mp, agno, idx, list[idx].busy_tp == tp);
+ trace_xfs_alloc_unbusy(tp->t_mountp, agno, idx, list[idx].busy_tp == tp);
if (list[idx].busy_tp == tp) {
list[idx].busy_tp = NULL;
- mp->m_perag[agno].pagb_count--;
+ pag->pagb_count--;
}
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
+ xfs_perag_put(pag);
}
@@ -2560,21 +2570,20 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
xfs_agblock_t bno,
xfs_extlen_t len)
{
- xfs_mount_t *mp;
+ struct xfs_perag *pag;
xfs_perag_busy_t *bsy;
xfs_agblock_t uend, bend;
xfs_lsn_t lsn;
int cnt;
- mp = tp->t_mountp;
-
- spin_lock(&mp->m_perag[agno].pagb_lock);
- cnt = mp->m_perag[agno].pagb_count;
+ pag = xfs_perag_get(tp->t_mountp, agno);
+ spin_lock(&pag->pagb_lock);
+ cnt = pag->pagb_count;
uend = bno + len - 1;
/* search pagb_list for this slot, skipping open slots */
- for (bsy = mp->m_perag[agno].pagb_list; cnt; bsy++) {
+ for (bsy = pag->pagb_list; cnt; bsy++) {
/*
* (start1,length1) within (start2, length2)
@@ -2589,7 +2598,7 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
}
}
- trace_xfs_alloc_busysearch(mp, agno, bno, len, !!cnt);
+ trace_xfs_alloc_busysearch(tp->t_mountp, agno, bno, len, !!cnt);
/*
* If a block was found, force the log through the LSN of the
@@ -2597,9 +2606,10 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
*/
if (cnt) {
lsn = bsy->busy_tp->t_commit_lsn;
- spin_unlock(&mp->m_perag[agno].pagb_lock);
- xfs_log_force(mp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
+ spin_unlock(&pag->pagb_lock);
+ xfs_log_force(tp->t_mountp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
} else {
- spin_unlock(&mp->m_perag[agno].pagb_lock);
+ spin_unlock(&pag->pagb_lock);
}
+ xfs_perag_put(pag);
}
diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index adbd914..b726e10 100644
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -61,12 +61,14 @@ xfs_allocbt_set_root(
struct xfs_agf *agf = XFS_BUF_TO_AGF(agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
int btnum = cur->bc_btnum;
+ struct xfs_perag *pag = xfs_perag_get(cur->bc_mp, seqno);
ASSERT(ptr->s != 0);
agf->agf_roots[btnum] = ptr->s;
be32_add_cpu(&agf->agf_levels[btnum], inc);
- cur->bc_mp->m_perag[seqno].pagf_levels[btnum] += inc;
+ pag->pagf_levels[btnum] += inc;
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_ROOTS | XFS_AGF_LEVELS);
}
@@ -150,6 +152,7 @@ xfs_allocbt_update_lastrec(
{
struct xfs_agf *agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
+ struct xfs_perag *pag;
__be32 len;
int numrecs;
@@ -193,7 +196,9 @@ xfs_allocbt_update_lastrec(
}
agf->agf_longest = len;
- cur->bc_mp->m_perag[seqno].pagf_longest = be32_to_cpu(len);
+ pag = xfs_perag_get(cur->bc_mp, seqno);
+ pag->pagf_longest = be32_to_cpu(len);
+ xfs_perag_put(pag);
xfs_alloc_log_agf(cur->bc_tp, cur->bc_private.a.agbp, XFS_AGF_LONGEST);
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 14+ messages in thread* RE: [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code
2009-12-15 6:11 ` [PATCH 2/6] XFS: Don't directly reference m_perag in allocation code Dave Chinner
2009-12-20 4:24 ` Dave Chinner
@ 2009-12-22 14:18 ` Alex Elder
1 sibling, 0 replies; 14+ messages in thread
From: Alex Elder @ 2009-12-22 14:18 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
Dave Chinner wrote:
> Start abstracting the perag references so that the indexing of the
> structures is not directly coded into all the places that uses the
> perag structures. This will allow us to separate the use of the
> perag structure and the way it is indexed and hence avoid the known
> deadlocks related to growing a busy filesystem.
Looks good. I think I would have preferred that you
added the matching "put" calls at the time you converted
the references to use "get" calls (mentioned on the last
patch) because now I find myself ignoring the mismatches.
No big deal.
> Signed-off-by: Dave Chinner <david@fromorbit.com>
Reviewed-by: Alex Elder <aelder@sgi.com>
> ---
> fs/xfs/xfs_alloc.c | 81 +++++++++++++++++++++++++--------------------
> fs/xfs/xfs_alloc_btree.c | 9 ++++-
> 2 files changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
> index a1c65fc..3cb533c 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -1662,11 +1662,13 @@ xfs_free_ag_extent(
> xfs_agf_t *agf;
> xfs_perag_t *pag; /* per allocation group data */
>
> + pag = xfs_perag_get(mp, agno);
> + pag->pagf_freeblks += len;
> + xfs_perag_put(pag);
> +
> agf = XFS_BUF_TO_AGF(agbp);
> - pag = &mp->m_perag[agno];
> be32_add_cpu(&agf->agf_freeblks, len);
> xfs_trans_agblocks_delta(tp, len);
> - pag->pagf_freeblks += len;
> XFS_WANT_CORRUPTED_GOTO(
> be32_to_cpu(agf->agf_freeblks) <=
> be32_to_cpu(agf->agf_length),
> @@ -1969,7 +1971,8 @@ xfs_alloc_get_freelist(
> xfs_trans_brelse(tp, agflbp);
> if (be32_to_cpu(agf->agf_flfirst) == XFS_AGFL_SIZE(mp))
> agf->agf_flfirst = 0;
> - pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
> +
> + pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
> be32_add_cpu(&agf->agf_flcount, -1);
> xfs_trans_agflist_delta(tp, -1);
> pag->pagf_flcount--;
> @@ -2078,7 +2081,8 @@ xfs_alloc_put_freelist(
> be32_add_cpu(&agf->agf_fllast, 1);
> if (be32_to_cpu(agf->agf_fllast) == XFS_AGFL_SIZE(mp))
> agf->agf_fllast = 0;
> - pag = &mp->m_perag[be32_to_cpu(agf->agf_seqno)];
> +
> + pag = xfs_perag_get(mp, be32_to_cpu(agf->agf_seqno));
> be32_add_cpu(&agf->agf_flcount, 1);
> xfs_trans_agflist_delta(tp, 1);
> pag->pagf_flcount++;
> @@ -2089,6 +2093,7 @@ xfs_alloc_put_freelist(
> pag->pagf_btreeblks--;
> logflags |= XFS_AGF_BTREEBLKS;
> }
> + xfs_perag_put(pag);
>
> xfs_alloc_log_agf(tp, agbp, logflags);
>
> @@ -2152,7 +2157,6 @@ xfs_read_agf(
> xfs_trans_brelse(tp, *bpp);
> return XFS_ERROR(EFSCORRUPTED);
> }
> -
> XFS_BUF_SET_VTYPE_REF(*bpp, B_FS_AGF, XFS_AGF_REF);
> return 0;
> }
> @@ -2184,7 +2188,7 @@ xfs_alloc_read_agf(
> ASSERT(!XFS_BUF_GETERROR(*bpp));
>
> agf = XFS_BUF_TO_AGF(*bpp);
> - pag = &mp->m_perag[agno];
> + pag = xfs_perag_get(mp, agno);
> if (!pag->pagf_init) {
> pag->pagf_freeblks = be32_to_cpu(agf->agf_freeblks);
> pag->pagf_btreeblks = be32_to_cpu(agf->agf_btreeblks);
> @@ -2211,6 +2215,7 @@ xfs_alloc_read_agf(
> be32_to_cpu(agf->agf_levels[XFS_BTNUM_CNTi]));
> }
> #endif
> + xfs_perag_put(pag);
> return 0;
> }
>
> @@ -2271,7 +2276,7 @@ xfs_alloc_vextent(
> */
> args->agno = XFS_FSB_TO_AGNO(mp, args->fsbno);
> down_read(&mp->m_peraglock);
> - args->pag = &mp->m_perag[args->agno];
> + args->pag = xfs_perag_get(mp, args->agno);
> args->minleft = 0;
> error = xfs_alloc_fix_freelist(args, 0);
> args->minleft = minleft;
> @@ -2341,7 +2346,7 @@ xfs_alloc_vextent(
> */
> down_read(&mp->m_peraglock);
> for (;;) {
> - args->pag = &mp->m_perag[args->agno];
> + args->pag = xfs_perag_get(mp, args->agno);
> if (no_min) args->minleft = 0;
> error = xfs_alloc_fix_freelist(args, flags);
> args->minleft = minleft;
> @@ -2400,6 +2405,7 @@ xfs_alloc_vextent(
> }
> }
> }
> + xfs_perag_put(args->pag);
> }
> up_read(&mp->m_peraglock);
> if (bump_rotor || (type == XFS_ALLOCTYPE_ANY_AG)) {
> @@ -2427,8 +2433,10 @@ xfs_alloc_vextent(
> args->len);
> #endif
> }
> + xfs_perag_put(args->pag);
> return 0;
> error0:
> + xfs_perag_put(args->pag);
> up_read(&mp->m_peraglock);
> return error;
> }
> @@ -2455,7 +2463,7 @@ xfs_free_extent(
> ASSERT(args.agno < args.mp->m_sb.sb_agcount);
> args.agbno = XFS_FSB_TO_AGBNO(args.mp, bno);
> down_read(&args.mp->m_peraglock);
> - args.pag = &args.mp->m_perag[args.agno];
> + args.pag = xfs_perag_get(args.mp, args.agno);
> if ((error = xfs_alloc_fix_freelist(&args, XFS_ALLOC_FLAG_FREEING)))
> goto error0;
> #ifdef DEBUG
> @@ -2465,6 +2473,7 @@ xfs_free_extent(
> #endif
> error = xfs_free_ag_extent(tp, args.agbp, args.agno, args.agbno, len, 0);
> error0:
> + xfs_perag_put(args.pag);
> up_read(&args.mp->m_peraglock);
> return error;
> }
> @@ -2486,15 +2495,15 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
> xfs_agblock_t bno,
> xfs_extlen_t len)
> {
> - xfs_mount_t *mp;
> xfs_perag_busy_t *bsy;
> + struct xfs_perag *pag;
> int n;
>
> - mp = tp->t_mountp;
> - spin_lock(&mp->m_perag[agno].pagb_lock);
> + pag = xfs_perag_get(tp->t_mountp, agno);
> + spin_lock(&pag->pagb_lock);
>
> /* search pagb_list for an open slot */
> - for (bsy = mp->m_perag[agno].pagb_list, n = 0;
> + for (bsy = pag->pagb_list, n = 0;
> n < XFS_PAGB_NUM_SLOTS;
> bsy++, n++) {
> if (bsy->busy_tp == NULL) {
> @@ -2502,11 +2511,11 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
> }
> }
>
> - trace_xfs_alloc_busy(mp, agno, bno, len, n);
> + trace_xfs_alloc_busy(tp->t_mountp, agno, bno, len, n);
>
> if (n < XFS_PAGB_NUM_SLOTS) {
> - bsy = &mp->m_perag[agno].pagb_list[n];
> - mp->m_perag[agno].pagb_count++;
> + bsy = &pag->pagb_list[n];
> + pag->pagb_count++;
> bsy->busy_start = bno;
> bsy->busy_length = len;
> bsy->busy_tp = tp;
> @@ -2521,7 +2530,8 @@ xfs_alloc_mark_busy(xfs_trans_t *tp,
> xfs_trans_set_sync(tp);
> }
>
> - spin_unlock(&mp->m_perag[agno].pagb_lock);
> + spin_unlock(&pag->pagb_lock);
> + xfs_perag_put(pag);
> }
>
> void
> @@ -2529,24 +2539,23 @@ xfs_alloc_clear_busy(xfs_trans_t *tp,
> xfs_agnumber_t agno,
> int idx)
> {
> - xfs_mount_t *mp;
> + struct xfs_perag *pag;
> xfs_perag_busy_t *list;
>
> - mp = tp->t_mountp;
> -
> - spin_lock(&mp->m_perag[agno].pagb_lock);
> - list = mp->m_perag[agno].pagb_list;
> -
> ASSERT(idx < XFS_PAGB_NUM_SLOTS);
> + pag = xfs_perag_get(tp->t_mountp, agno);
> + spin_lock(&pag->pagb_lock);
> + list = pag->pagb_list;
>
> - trace_xfs_alloc_unbusy(mp, agno, idx, list[idx].busy_tp == tp);
> + trace_xfs_alloc_unbusy(tp->t_mountp, agno, idx, list[idx].busy_tp == tp);
>
> if (list[idx].busy_tp == tp) {
> list[idx].busy_tp = NULL;
> - mp->m_perag[agno].pagb_count--;
> + pag->pagb_count--;
> }
>
> - spin_unlock(&mp->m_perag[agno].pagb_lock);
> + spin_unlock(&pag->pagb_lock);
> + xfs_perag_put(pag);
> }
>
>
> @@ -2560,21 +2569,20 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
> xfs_agblock_t bno,
> xfs_extlen_t len)
> {
> - xfs_mount_t *mp;
> + struct xfs_perag *pag;
> xfs_perag_busy_t *bsy;
> xfs_agblock_t uend, bend;
> xfs_lsn_t lsn;
> int cnt;
>
> - mp = tp->t_mountp;
> -
> - spin_lock(&mp->m_perag[agno].pagb_lock);
> - cnt = mp->m_perag[agno].pagb_count;
> + pag = xfs_perag_get(tp->t_mountp, agno);
> + spin_lock(&pag->pagb_lock);
> + cnt = pag->pagb_count;
>
> uend = bno + len - 1;
>
> /* search pagb_list for this slot, skipping open slots */
> - for (bsy = mp->m_perag[agno].pagb_list; cnt; bsy++) {
> + for (bsy = pag->pagb_list; cnt; bsy++) {
>
> /*
> * (start1,length1) within (start2, length2)
> @@ -2589,7 +2597,7 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
> }
> }
>
> - trace_xfs_alloc_busysearch(mp, agno, bno, len, !!cnt);
> + trace_xfs_alloc_busysearch(tp->t_mountp, agno, bno, len, !!cnt);
>
> /*
> * If a block was found, force the log through the LSN of the
> @@ -2597,9 +2605,10 @@ xfs_alloc_search_busy(xfs_trans_t *tp,
> */
> if (cnt) {
> lsn = bsy->busy_tp->t_commit_lsn;
> - spin_unlock(&mp->m_perag[agno].pagb_lock);
> - xfs_log_force(mp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
> + spin_unlock(&pag->pagb_lock);
> + xfs_log_force(tp->t_mountp, lsn, XFS_LOG_FORCE|XFS_LOG_SYNC);
> } else {
> - spin_unlock(&mp->m_perag[agno].pagb_lock);
> + spin_unlock(&pag->pagb_lock);
> }
> + xfs_perag_put(pag);
> }
> diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
> index adbd914..b726e10 100644
> --- a/fs/xfs/xfs_alloc_btree.c
> +++ b/fs/xfs/xfs_alloc_btree.c
> @@ -61,12 +61,14 @@ xfs_allocbt_set_root(
> struct xfs_agf *agf = XFS_BUF_TO_AGF(agbp);
> xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
> int btnum = cur->bc_btnum;
> + struct xfs_perag *pag = xfs_perag_get(cur->bc_mp, seqno);
>
> ASSERT(ptr->s != 0);
>
> agf->agf_roots[btnum] = ptr->s;
> be32_add_cpu(&agf->agf_levels[btnum], inc);
> - cur->bc_mp->m_perag[seqno].pagf_levels[btnum] += inc;
> + pag->pagf_levels[btnum] += inc;
> + xfs_perag_put(pag);
>
> xfs_alloc_log_agf(cur->bc_tp, agbp, XFS_AGF_ROOTS | XFS_AGF_LEVELS);
> }
> @@ -150,6 +152,7 @@ xfs_allocbt_update_lastrec(
> {
> struct xfs_agf *agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
> xfs_agnumber_t seqno = be32_to_cpu(agf->agf_seqno);
> + struct xfs_perag *pag;
> __be32 len;
> int numrecs;
>
> @@ -193,7 +196,9 @@ xfs_allocbt_update_lastrec(
> }
>
> agf->agf_longest = len;
> - cur->bc_mp->m_perag[seqno].pagf_longest = be32_to_cpu(len);
> + pag = xfs_perag_get(cur->bc_mp, seqno);
> + pag->pagf_longest = be32_to_cpu(len);
> + xfs_perag_put(pag);
> xfs_alloc_log_agf(cur->bc_tp, cur->bc_private.a.agbp, XFS_AGF_LONGEST);
> }
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 14+ messages in thread