xfs: untangle xfs_dialloc

Clarify the control flow in xfs_dialloc. Factor out a helper to go to the next node from the current one and improve the control flow by expanding composite if statements and using gotos. The xfs_ialloc_next_rec helper is borrowed from Dave Chinners dynamic allocation policy patches. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Alex Elder <aelder@sgi.com> Signed-off-by: Felix Blyakher <felixb@sgi.com>

xfs: untangle xfs_dialloc
Clarify the control flow in xfs_dialloc. Factor out a helper to go to the next node from the current one and improve the control flow by expanding composite if statements and using gotos. The xfs_ialloc_next_rec helper is borrowed from Dave Chinners dynamic allocation policy patches. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Alex Elder <aelder@sgi.com> Signed-off-by: Felix Blyakher <felixb@sgi.com>
Christoph Hellwig · Felix Blyakher
1 parent 0b48db80ba
Showing 1 changed file with 138 additions and 153 deletions Side-by-side Diff
fs/xfs/xfs_ialloc.c
@@ -589,7 +589,38 @@
 	}
 }
  
+/*
+ * Try to retrieve the next record to the left/right from the current one.
+ */
+STATIC int
+xfs_ialloc_next_rec(
+	struct xfs_btree_cur	*cur,
+	xfs_inobt_rec_incore_t	*rec,
+	int			*done,
+	int			left)
+{
+	int                     error;
+	int			i;
  
+	if (left)
+		error = xfs_btree_decrement(cur, 0, &i);
+	else
+		error = xfs_btree_increment(cur, 0, &i);
+
+	if (error)
+		return error;
+	*done = !i;
+	if (i) {
+		error = xfs_inobt_get_rec(cur, rec, &i);
+		if (error)
+			return error;
+		XFS_WANT_CORRUPTED_RETURN(i == 1);
+	}
+
+	return 0;
+}
+
+
 /*
  * Visible inode allocation functions.
  */
@@ -644,8 +675,8 @@
 	int		j;		/* result code */
 	xfs_mount_t	*mp;		/* file system mount structure */
 	int		offset;		/* index of inode in chunk */
-	xfs_agino_t	pagino;		/* parent's a.g. relative inode # */
-	xfs_agnumber_t	pagno;		/* parent's allocation group number */
+	xfs_agino_t	pagino;		/* parent's AG relative inode # */
+	xfs_agnumber_t	pagno;		/* parent's AG number */
 	xfs_inobt_rec_incore_t rec;	/* inode allocation record */
 	xfs_agnumber_t	tagno;		/* testing allocation group number */
 	xfs_btree_cur_t	*tcur;		/* temp cursor */
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
  
@@ -781,186 +812,140 @@
 		goto error0;
  
 	/*
-	 * If in the same a.g. as the parent, try to get near the parent.
+	 * If in the same AG as the parent, try to get near the parent.
 	 */
 	if (pagno == agno) {
-		if ((error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i)))
+		int		doneleft;	/* done, to the left */
+		int		doneright;	/* done, to the right */
+
+		error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i);
+		if (error)
 			goto error0;
-		if (i != 0 &&
-		    (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
-		    j == 1 &&
-		    rec.ir_freecount > 0) {
+		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+		error = xfs_inobt_get_rec(cur, &rec, &j);
+		if (error)
+			goto error0;
+		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+		if (rec.ir_freecount > 0) {
 			/*
 			 * Found a free inode in the same chunk
-			 * as parent, done.
+			 * as the parent, done.
 			 */
+			goto alloc_inode;
 		}
+
+
 		/*
-		 * In the same a.g. as parent, but parent's chunk is full.
+		 * In the same AG as parent, but parent's chunk is full.
 		 */
-		else {
-			int	doneleft;	/* done, to the left */
-			int	doneright;	/* done, to the right */
  
-			if (error)
-				goto error0;
-			ASSERT(i == 1);
-			ASSERT(j == 1);
-			/*
-			 * Duplicate the cursor, search left & right
-			 * simultaneously.
-			 */
-			if ((error = xfs_btree_dup_cursor(cur, &tcur)))
-				goto error0;
-			/*
-			 * Search left with tcur, back up 1 record.
-			 */
-			if ((error = xfs_btree_decrement(tcur, 0, &i)))
-				goto error1;
-			doneleft = !i;
-			if (!doneleft) {
-				error = xfs_inobt_get_rec(tcur, &trec, &i);
-				if (error)
-					goto error1;
-				XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+		/* duplicate the cursor, search left & right simultaneously */
+		error = xfs_btree_dup_cursor(cur, &tcur);
+		if (error)
+			goto error0;
+
+		/* search left with tcur, back up 1 record */
+		error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
+		if (error)
+			goto error1;
+
+		/* search right with cur, go forward 1 record. */
+		error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
+		if (error)
+			goto error1;
+
+		/*
+		 * Loop until we find an inode chunk with a free inode.
+		 */
+		while (!doneleft || !doneright) {
+			int	useleft;  /* using left inode chunk this time */
+
+			/* figure out the closer block if both are valid. */
+			if (!doneleft && !doneright) {
+				useleft = pagino -
+				 (trec.ir_startino + XFS_INODES_PER_CHUNK - 1) <
+				  rec.ir_startino - pagino;
+			} else {
+				useleft = !doneleft;
 			}
-			/*
-			 * Search right with cur, go forward 1 record.
-			 */
-			if ((error = xfs_btree_increment(cur, 0, &i)))
-				goto error1;
-			doneright = !i;
-			if (!doneright) {
-				error = xfs_inobt_get_rec(cur, &rec, &i);
-				if (error)
-					goto error1;
-				XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
+
+			/* free inodes to the left? */
+			if (useleft && trec.ir_freecount) {
+				rec = trec;
+				xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
+				cur = tcur;
+				goto alloc_inode;
 			}
-			/*
-			 * Loop until we find the closest inode chunk
-			 * with a free one.
-			 */
-			while (!doneleft || !doneright) {
-				int	useleft;  /* using left inode
-						     chunk this time */
  
-				/*
-				 * Figure out which block is closer,
-				 * if both are valid.
-				 */
-				if (!doneleft && !doneright)
-					useleft =
-						pagino -
-						(trec.ir_startino +
-						 XFS_INODES_PER_CHUNK - 1) <
-						 rec.ir_startino - pagino;
-				else
-					useleft = !doneleft;
-				/*
-				 * If checking the left, does it have
-				 * free inodes?
-				 */
-				if (useleft && trec.ir_freecount) {
-					/*
-					 * Yes, set it up as the chunk to use.
-					 */
-					rec = trec;
-					xfs_btree_del_cursor(cur,
-						XFS_BTREE_NOERROR);
-					cur = tcur;
-					break;
-				}
-				/*
-				 * If checking the right, does it have
-				 * free inodes?
-				 */
-				if (!useleft && rec.ir_freecount) {
-					/*
-					 * Yes, it's already set up.
-					 */
-					xfs_btree_del_cursor(tcur,
-						XFS_BTREE_NOERROR);
-					break;
-				}
-				/*
-				 * If used the left, get another one
-				 * further left.
-				 */
-				if (useleft) {
-					if ((error = xfs_btree_decrement(tcur, 0,
-							&i)))
-						goto error1;
-					doneleft = !i;
-					if (!doneleft) {
-						error = xfs_inobt_get_rec(
-							    tcur, &trec, &i);
-						if (error)
-							goto error1;
-						XFS_WANT_CORRUPTED_GOTO(i == 1,
-							error1);
-					}
-				}
-				/*
-				 * If used the right, get another one
-				 * further right.
-				 */
-				else {
-					if ((error = xfs_btree_increment(cur, 0,
-							&i)))
-						goto error1;
-					doneright = !i;
-					if (!doneright) {
-						error = xfs_inobt_get_rec(
-							    cur, &rec, &i);
-						if (error)
-							goto error1;
-						XFS_WANT_CORRUPTED_GOTO(i == 1,
-							error1);
-					}
-				}
+			/* free inodes to the right? */
+			if (!useleft && rec.ir_freecount) {
+				xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
+				goto alloc_inode;
 			}
-			ASSERT(!doneleft || !doneright);
+
+			/* get next record to check */
+			if (useleft) {
+				error = xfs_ialloc_next_rec(tcur, &trec,
+								 &doneleft, 1);
+			} else {
+				error = xfs_ialloc_next_rec(cur, &rec,
+								 &doneright, 0);
+			}
+			if (error)
+				goto error1;
 		}
+		ASSERT(!doneleft || !doneright);
 	}
+
 	/*
-	 * In a different a.g. from the parent.
+	 * In a different AG from the parent.
 	 * See if the most recently allocated block has any free.
 	 */
 	else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
-		if ((error = xfs_inobt_lookup_eq(cur,
-				be32_to_cpu(agi->agi_newino), 0, 0, &i)))
+		error = xfs_inobt_lookup_eq(cur, be32_to_cpu(agi->agi_newino),
+					    0, 0, &i);
+		if (error)
 			goto error0;
-		if (i == 1 &&
-		    (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
-		    j == 1 &&
-		    rec.ir_freecount > 0) {
-			/*
-			 * The last chunk allocated in the group still has
-			 * a free inode.
-			 */
+
+		if (i == 1) {
+			error = xfs_inobt_get_rec(cur, &rec, &j);
+			if (error)
+				goto error0;
+
+			if (j == 1 && rec.ir_freecount > 0) {
+				/*
+				 * The last chunk allocated in the group
+				 * still has a free inode.
+				 */
+				goto alloc_inode;
+			}
 		}
+
 		/*
-		 * None left in the last group, search the whole a.g.
+		 * None left in the last group, search the whole AG
 		 */
-		else {
+		error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
+		if (error)
+			goto error0;
+		XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+
+		for (;;) {
+			error = xfs_inobt_get_rec(cur, &rec, &i);
 			if (error)
 				goto error0;
-			if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
+			XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
+			if (rec.ir_freecount > 0)
+				break;
+			error = xfs_btree_increment(cur, 0, &i);
+			if (error)
 				goto error0;
-			ASSERT(i == 1);
-			for (;;) {
-				error = xfs_inobt_get_rec(cur, &rec, &i);
-				if (error)
-					goto error0;
-				XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-				if (rec.ir_freecount > 0)
-					break;
-				if ((error = xfs_btree_increment(cur, 0, &i)))
-					goto error0;
-				XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
-			}
+			XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
 		}
 	}
+
+alloc_inode:
 	offset = xfs_ialloc_find_free(&rec.ir_free);
 	ASSERT(offset >= 0);
 	ASSERT(offset < XFS_INODES_PER_CHUNK);
...	...	@@ -589,7 +589,38 @@
589	589	}
590	590	}
591	591
	592	+/*
	593	+ * Try to retrieve the next record to the left/right from the current one.
	594	+ */
	595	+STATIC int
	596	+xfs_ialloc_next_rec(
	597	+ struct xfs_btree_cur *cur,
	598	+ xfs_inobt_rec_incore_t *rec,
	599	+ int *done,
	600	+ int left)
	601	+{
	602	+ int error;
	603	+ int i;
592	604
	605	+ if (left)
	606	+ error = xfs_btree_decrement(cur, 0, &i);
	607	+ else
	608	+ error = xfs_btree_increment(cur, 0, &i);
	609	+
	610	+ if (error)
	611	+ return error;
	612	+ *done = !i;
	613	+ if (i) {
	614	+ error = xfs_inobt_get_rec(cur, rec, &i);
	615	+ if (error)
	616	+ return error;
	617	+ XFS_WANT_CORRUPTED_RETURN(i == 1);
	618	+ }
	619	+
	620	+ return 0;
	621	+}
	622	+
	623	+
593	624	/*
594	625	* Visible inode allocation functions.
595	626	*/
...	...	@@ -644,8 +675,8 @@
644	675	int j; /* result code */
645	676	xfs_mount_t mp; / file system mount structure */
646	677	int offset; /* index of inode in chunk */
647		- xfs_agino_t pagino; /* parent's a.g. relative inode # */
648		- xfs_agnumber_t pagno; /* parent's allocation group number */
	678	+ xfs_agino_t pagino; /* parent's AG relative inode # */
	679	+ xfs_agnumber_t pagno; /* parent's AG number */
649	680	xfs_inobt_rec_incore_t rec; /* inode allocation record */
650	681	xfs_agnumber_t tagno; /* testing allocation group number */
651	682	xfs_btree_cur_t tcur; / temp cursor */
652	683
653	684
654	685
655	686
656	687
657	688
658	689
659	690
660	691
661	692
662	693
663	694
664	695
665	696
666	697
667	698
668	699
669	700
670	701
671	702
672	703
673	704
674	705
...	...	@@ -781,186 +812,140 @@
781	812	goto error0;
782	813
783	814	/*
784		- * If in the same a.g. as the parent, try to get near the parent.
	815	+ * If in the same AG as the parent, try to get near the parent.
785	816	*/
786	817	if (pagno == agno) {
787		- if ((error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i)))
	818	+ int doneleft; /* done, to the left */
	819	+ int doneright; /* done, to the right */
	820	+
	821	+ error = xfs_inobt_lookup_le(cur, pagino, 0, 0, &i);
	822	+ if (error)
788	823	goto error0;
789		- if (i != 0 &&
790		- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
791		- j == 1 &&
792		- rec.ir_freecount > 0) {
	824	+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
	825	+
	826	+ error = xfs_inobt_get_rec(cur, &rec, &j);
	827	+ if (error)
	828	+ goto error0;
	829	+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
	830	+
	831	+ if (rec.ir_freecount > 0) {
793	832	/*
794	833	* Found a free inode in the same chunk
795		- * as parent, done.
	834	+ * as the parent, done.
796	835	*/
	836	+ goto alloc_inode;
797	837	}
	838	+
	839	+
798	840	/*
799		- * In the same a.g. as parent, but parent's chunk is full.
	841	+ * In the same AG as parent, but parent's chunk is full.
800	842	*/
801		- else {
802		- int doneleft; /* done, to the left */
803		- int doneright; /* done, to the right */
804	843
805		- if (error)
806		- goto error0;
807		- ASSERT(i == 1);
808		- ASSERT(j == 1);
809		- /*
810		- * Duplicate the cursor, search left & right
811		- * simultaneously.
812		- */
813		- if ((error = xfs_btree_dup_cursor(cur, &tcur)))
814		- goto error0;
815		- /*
816		- * Search left with tcur, back up 1 record.
817		- */
818		- if ((error = xfs_btree_decrement(tcur, 0, &i)))
819		- goto error1;
820		- doneleft = !i;
821		- if (!doneleft) {
822		- error = xfs_inobt_get_rec(tcur, &trec, &i);
823		- if (error)
824		- goto error1;
825		- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
	844	+ /* duplicate the cursor, search left & right simultaneously */
	845	+ error = xfs_btree_dup_cursor(cur, &tcur);
	846	+ if (error)
	847	+ goto error0;
	848	+
	849	+ /* search left with tcur, back up 1 record */
	850	+ error = xfs_ialloc_next_rec(tcur, &trec, &doneleft, 1);
	851	+ if (error)
	852	+ goto error1;
	853	+
	854	+ /* search right with cur, go forward 1 record. */
	855	+ error = xfs_ialloc_next_rec(cur, &rec, &doneright, 0);
	856	+ if (error)
	857	+ goto error1;
	858	+
	859	+ /*
	860	+ * Loop until we find an inode chunk with a free inode.
	861	+ */
	862	+ while (!doneleft \|\| !doneright) {
	863	+ int useleft; /* using left inode chunk this time */
	864	+
	865	+ /* figure out the closer block if both are valid. */
	866	+ if (!doneleft && !doneright) {
	867	+ useleft = pagino -
	868	+ (trec.ir_startino + XFS_INODES_PER_CHUNK - 1) <
	869	+ rec.ir_startino - pagino;
	870	+ } else {
	871	+ useleft = !doneleft;
826	872	}
827		- /*
828		- * Search right with cur, go forward 1 record.
829		- */
830		- if ((error = xfs_btree_increment(cur, 0, &i)))
831		- goto error1;
832		- doneright = !i;
833		- if (!doneright) {
834		- error = xfs_inobt_get_rec(cur, &rec, &i);
835		- if (error)
836		- goto error1;
837		- XFS_WANT_CORRUPTED_GOTO(i == 1, error1);
	873	+
	874	+ /* free inodes to the left? */
	875	+ if (useleft && trec.ir_freecount) {
	876	+ rec = trec;
	877	+ xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
	878	+ cur = tcur;
	879	+ goto alloc_inode;
838	880	}
839		- /*
840		- * Loop until we find the closest inode chunk
841		- * with a free one.
842		- */
843		- while (!doneleft \|\| !doneright) {
844		- int useleft; /* using left inode
845		- chunk this time */
846	881
847		- /*
848		- * Figure out which block is closer,
849		- * if both are valid.
850		- */
851		- if (!doneleft && !doneright)
852		- useleft =
853		- pagino -
854		- (trec.ir_startino +
855		- XFS_INODES_PER_CHUNK - 1) <
856		- rec.ir_startino - pagino;
857		- else
858		- useleft = !doneleft;
859		- /*
860		- * If checking the left, does it have
861		- * free inodes?
862		- */
863		- if (useleft && trec.ir_freecount) {
864		- /*
865		- * Yes, set it up as the chunk to use.
866		- */
867		- rec = trec;
868		- xfs_btree_del_cursor(cur,
869		- XFS_BTREE_NOERROR);
870		- cur = tcur;
871		- break;
872		- }
873		- /*
874		- * If checking the right, does it have
875		- * free inodes?
876		- */
877		- if (!useleft && rec.ir_freecount) {
878		- /*
879		- * Yes, it's already set up.
880		- */
881		- xfs_btree_del_cursor(tcur,
882		- XFS_BTREE_NOERROR);
883		- break;
884		- }
885		- /*
886		- * If used the left, get another one
887		- * further left.
888		- */
889		- if (useleft) {
890		- if ((error = xfs_btree_decrement(tcur, 0,
891		- &i)))
892		- goto error1;
893		- doneleft = !i;
894		- if (!doneleft) {
895		- error = xfs_inobt_get_rec(
896		- tcur, &trec, &i);
897		- if (error)
898		- goto error1;
899		- XFS_WANT_CORRUPTED_GOTO(i == 1,
900		- error1);
901		- }
902		- }
903		- /*
904		- * If used the right, get another one
905		- * further right.
906		- */
907		- else {
908		- if ((error = xfs_btree_increment(cur, 0,
909		- &i)))
910		- goto error1;
911		- doneright = !i;
912		- if (!doneright) {
913		- error = xfs_inobt_get_rec(
914		- cur, &rec, &i);
915		- if (error)
916		- goto error1;
917		- XFS_WANT_CORRUPTED_GOTO(i == 1,
918		- error1);
919		- }
920		- }
	882	+ /* free inodes to the right? */
	883	+ if (!useleft && rec.ir_freecount) {
	884	+ xfs_btree_del_cursor(tcur, XFS_BTREE_NOERROR);
	885	+ goto alloc_inode;
921	886	}
922		- ASSERT(!doneleft \|\| !doneright);
	887	+
	888	+ /* get next record to check */
	889	+ if (useleft) {
	890	+ error = xfs_ialloc_next_rec(tcur, &trec,
	891	+ &doneleft, 1);
	892	+ } else {
	893	+ error = xfs_ialloc_next_rec(cur, &rec,
	894	+ &doneright, 0);
	895	+ }
	896	+ if (error)
	897	+ goto error1;
923	898	}
	899	+ ASSERT(!doneleft \|\| !doneright);
924	900	}
	901	+
925	902	/*
926		- * In a different a.g. from the parent.
	903	+ * In a different AG from the parent.
927	904	* See if the most recently allocated block has any free.
928	905	*/
929	906	else if (be32_to_cpu(agi->agi_newino) != NULLAGINO) {
930		- if ((error = xfs_inobt_lookup_eq(cur,
931		- be32_to_cpu(agi->agi_newino), 0, 0, &i)))
	907	+ error = xfs_inobt_lookup_eq(cur, be32_to_cpu(agi->agi_newino),
	908	+ 0, 0, &i);
	909	+ if (error)
932	910	goto error0;
933		- if (i == 1 &&
934		- (error = xfs_inobt_get_rec(cur, &rec, &j)) == 0 &&
935		- j == 1 &&
936		- rec.ir_freecount > 0) {
937		- /*
938		- * The last chunk allocated in the group still has
939		- * a free inode.
940		- */
	911	+
	912	+ if (i == 1) {
	913	+ error = xfs_inobt_get_rec(cur, &rec, &j);
	914	+ if (error)
	915	+ goto error0;
	916	+
	917	+ if (j == 1 && rec.ir_freecount > 0) {
	918	+ /*
	919	+ * The last chunk allocated in the group
	920	+ * still has a free inode.
	921	+ */
	922	+ goto alloc_inode;
	923	+ }
941	924	}
	925	+
942	926	/*
943		- * None left in the last group, search the whole a.g.
	927	+ * None left in the last group, search the whole AG
944	928	*/
945		- else {
	929	+ error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i);
	930	+ if (error)
	931	+ goto error0;
	932	+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
	933	+
	934	+ for (;;) {
	935	+ error = xfs_inobt_get_rec(cur, &rec, &i);
946	936	if (error)
947	937	goto error0;
948		- if ((error = xfs_inobt_lookup_ge(cur, 0, 0, 0, &i)))
	938	+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
	939	+ if (rec.ir_freecount > 0)
	940	+ break;
	941	+ error = xfs_btree_increment(cur, 0, &i);
	942	+ if (error)
949	943	goto error0;
950		- ASSERT(i == 1);
951		- for (;;) {
952		- error = xfs_inobt_get_rec(cur, &rec, &i);
953		- if (error)
954		- goto error0;
955		- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
956		- if (rec.ir_freecount > 0)
957		- break;
958		- if ((error = xfs_btree_increment(cur, 0, &i)))
959		- goto error0;
960		- XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
961		- }
	944	+ XFS_WANT_CORRUPTED_GOTO(i == 1, error0);
962	945	}
963	946	}
	947	+
	948	+alloc_inode:
964	949	offset = xfs_ialloc_find_free(&rec.ir_free);
965	950	ASSERT(offset >= 0);
966	951	ASSERT(offset < XFS_INODES_PER_CHUNK);