Skip to contentSkip to conversation

xfs: remove lazy per-AG initialization

PATCH In review Christoph Hellwig 3 participants 4 messages May 28, 2010 – Jun 4, 2010 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xfs: remove lazy per-AG initialization

Historically XFS initializes the allocator / inode allocator per-AG lazily, that is the first time this information is required. For filesystems that use lazy superblock counters (which is the default now) we already have to walk all AGs to initialize the superblock counters on an unclean shutdown. This patch generalizes that code so that we always initialize the per-AG data on mount, and also during growfs so that we can remove all the special case code in the fastpath which couldn't assume that the per-AG data is already initialized.

fs/xfs/xfs_ag.h +0 -2
Index: xfs/fs/xfs/xfs_ag.h
===================================================================
@@ -200,8 +200,6 @@ typedef struct xfs_perag {
 	struct xfs_mount *pag_mount;	/* owner filesystem */
 	xfs_agnumber_t	pag_agno;	/* AG this structure belongs to */
 	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 */
 	char		pagi_inodeok;	/* The agi is ok for inodes */
 	__uint8_t	pagf_levels[XFS_BTNUM_AGF];
fs/xfs/xfs_alloc.c +35 -53

141 lines not on this page · raw message

fs/xfs/xfs_alloc.h +2 -7
Index: xfs/fs/xfs/xfs_alloc.h
===================================================================
@@ -158,14 +158,9 @@ xfs_alloc_log_agf(
 	int		fields);/* mask of fields to be logged (XFS_AGF_...) */
 
 /*
- * Interface for inode allocation to force the pag data to be initialized.
+ * Read in the AGF to initialise the per-AG data in the mount structure
  */
-int				/* error */
-xfs_alloc_pagf_init(
-	struct xfs_mount *mp,	/* file system mount structure */
-	struct xfs_trans *tp,	/* transaction pointer */
-	xfs_agnumber_t	agno,	/* allocation group number */
-	int		flags);	/* XFS_ALLOC_FLAGS_... */
+int xfs_alloc_pagf_init(struct xfs_mount *mp, struct xfs_perag *pag);
 
 /*
  * Put the block on the freelist for the allocation group.
fs/xfs/xfs_bmap.c +5 -17
Index: xfs/fs/xfs/xfs_bmap.c
===================================================================
@@ -2552,7 +2552,6 @@ xfs_bmap_btalloc_nullfb(
 	struct xfs_mount	*mp = ap->ip->i_mount;
 	struct xfs_perag	*pag;
 	xfs_agnumber_t		ag, startag;
-	int			notinit = 0;
 	int			error;
 
 	if (ap->userdata && xfs_inode_is_filestream(ap->ip))
@@ -2572,25 +2571,14 @@ xfs_bmap_btalloc_nullfb(
 
 	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);
-			if (error) {
-				xfs_perag_put(pag);
-				return error;
-			}
-		}
+		xfs_extlen_t		longest;
 
 		/*
 		 * See xfs_alloc_fix_freelist...
 		 */
-		if (pag->pagf_init) {
-			xfs_extlen_t	longest;
-			longest = xfs_alloc_longest_free_extent(mp, pag);
-			if (*blen < longest)
-				*blen = longest;
-		} else
-			notinit = 1;
+		longest = xfs_alloc_longest_free_extent(mp, pag);
+		if (*blen < longest)
+			*blen = longest;
 
 		if (xfs_inode_is_filestream(ap->ip)) {
 			if (*blen >= ap->alen)
@@ -2633,7 +2621,7 @@ xfs_bmap_btalloc_nullfb(
 	 * Since the above loop did a BUF_TRYLOCK, it is
 	 * possible that there is space for this request.
 	 */
-	if (notinit || *blen < ap->minlen)
+	if (*blen < ap->minlen)
 		args->minlen = ap->minlen;
 	/*
 	 * If the best seen length is less than the request
fs/xfs/xfs_filestream.c +1 -22
Index: xfs/fs/xfs/xfs_filestream.c
===================================================================
@@ -137,7 +137,7 @@ _xfs_filestream_pick_ag(
 	xfs_extlen_t	minlen)
 {
 	int		streams, max_streams;
-	int		err, trylock, nscan;
+	int		nscan;
 	xfs_extlen_t	longest, free, minfree, maxfree = 0;
 	xfs_agnumber_t	ag, max_ag = NULLAGNUMBER;
 	struct xfs_perag *pag;
@@ -148,25 +148,10 @@ _xfs_filestream_pick_ag(
 	ag = startag;
 	*agp = NULLAGNUMBER;
 
-	/* For the first pass, don't sleep trying to init the per-AG. */
-	trylock = XFS_ALLOC_FLAG_TRYLOCK;
-
 	for (nscan = 0; 1; nscan++) {
 		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) {
-				xfs_perag_put(pag);
-				return err;
-			}
-		}
-
-		/* Might fail sometimes during the 1st pass with trylock set. */
-		if (!pag->pagf_init)
-			goto next_ag;
-
 		/* Keep track of the AG with the most free blocks. */
 		if (pag->pagf_freeblks > maxfree) {
 			maxfree = pag->pagf_freeblks;
@@ -211,12 +196,6 @@ next_ag:
 		if (ag != startag)
 			continue;
 
-		/* Allow sleeping in xfs_alloc_pagf_init() on the 2nd pass. */
-		if (trylock != 0) {
-			trylock = 0;
-			continue;
-		}
-
 		/* Finally, if lowspace wasn't set, set it for the 3rd pass. */
 		if (!(flags & XFS_PICK_LOWSPACE)) {
 			flags |= XFS_PICK_LOWSPACE;
fs/xfs/xfs_ialloc.c +44 -64

159 lines not on this page · raw message

fs/xfs/xfs_ialloc.h +2 -7
Index: xfs/fs/xfs/xfs_ialloc.h
===================================================================
@@ -140,14 +140,9 @@ xfs_ialloc_read_agi(
 	struct xfs_buf	**bpp);		/* allocation group hdr buf */
 
 /*
- * Read in the allocation group header to initialise the per-ag data
- * in the mount structure
+ * Read in the AGI to initialise the per-AG data in the mount structure
  */
-int
-xfs_ialloc_pagi_init(
-	struct xfs_mount *mp,		/* file system mount structure */
-	struct xfs_trans *tp,		/* transaction pointer */
-        xfs_agnumber_t  agno);		/* allocation group number */
+int xfs_ialloc_pagi_init(struct xfs_mount *, struct xfs_perag *);
 
 /*
  * Lookup a record by ino in the btree given by cur.
fs/xfs/xfs_mount.c +66 -60

154 lines not on this page · raw message

fs/xfs/xfs_fsops.c +11 -0
Index: xfs/fs/xfs/xfs_fsops.c
===================================================================
@@ -186,6 +186,8 @@ xfs_growfs_data_private(
 	 */
 	nfree = 0;
 	for (agno = nagcount - 1; agno >= oagcount; agno--, new -= agsize) {
+		struct xfs_perag	*pag;
+
 		/*
 		 * AG freelist header block
 		 */
@@ -305,6 +307,15 @@ xfs_growfs_data_private(
 		if (error) {
 			goto error0;
 		}
+
+		pag = xfs_perag_get(mp, agno);
+		error = xfs_alloc_pagf_init(mp, pag);
+		if (!error)
+			error = xfs_ialloc_pagi_init(mp, pag);
+		xfs_perag_put(pag);
+
+		if (error)
+			goto error0;
 	}
 	xfs_trans_agblocks_delta(tp, nfree);
 	/*

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 5 lines · Christoph Hellwig

Historically XFS initializes the allocator / inode allocator per-AG lazily, that is the first time this information is required. For filesystems that use lazy superblock counters (which is the default now) we already have to walk all AGs to initialize the superblock counters on an unclean shutdown.

Which is not common, so isn't frequently triggered in the normal mount process. The reason for the lazy initialisation is to speed the mount process up when there are thousands of AGs. That is, we avoid thousands of serialised IOs in the mount path. Have you checked to see what the impact is on the clean mount execution time is on such a filesystem?

FWIW, in the case of an unclean shutdown, we are already on the slow path due to log recovery so adding IO to read all the headers it not such a big deal as they have probably been read in during replay, anyway.

Quoted 4 lines

This patch generalizes that code so that we
always initialize the per-AG data on mount, and also during growfs so that we can remove all the special case code in the fastpath which couldn't assume that the per-AG data is already initialized.

I like the cleanup, but I'm not sure that potentially adding tens of seconds to the time to mount a really large filesystem is a good tradeoff...

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
#2 lore raw
Quoted 7 lines · Dave Chinner
Quoted 5 lines · Christoph Hellwig

Historically XFS initializes the allocator / inode allocator per-AG lazily, that is the first time this information is required. For filesystems that use lazy superblock counters (which is the default now) we already have to walk all AGs to initialize the superblock counters on an unclean shutdown.

Quoted 6 lines

Which is not common, so isn't frequently triggered in the normal mount process. The reason for the lazy initialisation is to speed the mount process up when there are thousands of AGs. That is, we avoid thousands of serialised IOs in the mount path. Have you checked to see what the impact is on the clean mount execution time is on such a filesystem?

It's interesting that the time penalty you're talking about doesn't go away, it just becomes less noticeable because it's aggregated over subsequent access to the AG's.

I like the cleanup too, but I agree it would be useful to quantify what this particular impact is.

-Alex

Quoted 9 lines

FWIW, in the case of an unclean shutdown, we are already on the slow path due to log recovery so adding IO to read all the headers it not such a big deal as they have probably been read in during replay, anyway.

Quoted 4 lines

This patch generalizes that code so that we
always initialize the per-AG data on mount, and also during growfs so that we can remove all the special case code in the fastpath which couldn't assume that the per-AG data is already initialized.

Quoted 7 lines

I like the cleanup, but I'm not sure that potentially adding tens of seconds to the time to mount a really large filesystem is a good tradeoff...

Cheers,

Dave.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw
Quoted 15 lines · Alex Elder
Quoted 7 lines · Dave Chinner
Quoted 5 lines · Christoph Hellwig

Historically XFS initializes the allocator / inode allocator per-AG lazily, that is the first time this information is required. For filesystems that use lazy superblock counters (which is the default now) we already have to walk all AGs to initialize the superblock counters on an unclean shutdown.

Quoted 6 lines

Which is not common, so isn't frequently triggered in the normal mount process. The reason for the lazy initialisation is to speed the mount process up when there are thousands of AGs. That is, we avoid thousands of serialised IOs in the mount path. Have you checked to see what the impact is on the clean mount execution time is on such a filesystem?

Quoted 3 lines

It's interesting that the time penalty you're talking about doesn't go away, it just becomes less noticeable because it's aggregated over subsequent access to the AG's.

Right, the penalty is currently taken at access time, rather than at mount time. One way to test the impact is to compare the runtime difference for xfstests with MKFS_OPTIONS="-d agsize=16m" to bump up the AG count and see how much additional IO and time it takes...

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs