xfs: bulkstat main loop logic is a mess

There are a bunch of variables tha tare more wildy scoped than they need to be, obfuscated user buffer checks and tortured "next inode" tracking. This all needs cleaning up to expose the real issues that need fixing. cc: <stable@vger.kernel.org> # 3.17 Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Dave Chinner <david@fromorbit.com>

xfs: bulkstat main loop logic is a mess
There are a bunch of variables tha tare more wildy scoped than they need to be, obfuscated user buffer checks and tortured "next inode" tracking. This all needs cleaning up to expose the real issues that need fixing. cc: <stable@vger.kernel.org> # 3.17 Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Brian Foster <bfoster@redhat.com> Signed-off-by: Dave Chinner <david@fromorbit.com>
Dave Chinner · Dave Chinner
1 parent 2b831ac6bc
Showing 1 changed file with 24 additions and 32 deletions Side-by-side Diff
fs/xfs/xfs_itable.c
@@ -348,30 +348,23 @@
 	xfs_agino_t		agino;	/* inode # in allocation group */
 	xfs_agnumber_t		agno;	/* allocation group number */
 	xfs_btree_cur_t		*cur;	/* btree cursor for ialloc btree */
-	int			end_of_ag; /* set if we've seen the ag end */
-	int			error;	/* error code */
-	int			icount;	/* count of inodes good in irbuf */
 	size_t			irbsize; /* size of irec buffer in bytes */
-	xfs_ino_t		ino;	/* inode number (filesystem) */
-	xfs_inobt_rec_incore_t	*irbp;	/* current irec buffer pointer */
 	xfs_inobt_rec_incore_t	*irbuf;	/* start of irec buffer */
-	xfs_inobt_rec_incore_t	*irbufend; /* end of good irec buffer entries */
 	xfs_ino_t		lastino; /* last inode number returned */
 	int			nirbuf;	/* size of irbuf */
 	int			rval;	/* return value error code */
 	int			ubcount; /* size of user's buffer */
-	int			stat;
 	struct xfs_bulkstat_agichunk ac;
+	int			error = 0;
  
 	/*
 	 * Get the last inode value, see if there's nothing to do.
 	 */
-	ino = (xfs_ino_t)*lastinop;
-	lastino = ino;
-	agno = XFS_INO_TO_AGNO(mp, ino);
-	agino = XFS_INO_TO_AGINO(mp, ino);
+	lastino = *lastinop;
+	agno = XFS_INO_TO_AGNO(mp, lastino);
+	agino = XFS_INO_TO_AGINO(mp, lastino);
 	if (agno >= mp->m_sb.sb_agcount ||
-	    ino != XFS_AGINO_TO_INO(mp, agno, agino)) {
+	    lastino != XFS_AGINO_TO_INO(mp, agno, agino)) {
 		*done = 1;
 		*ubcountp = 0;
 		return 0;
@@ -396,8 +389,13 @@
 	 * inode returned; 0 means start of the allocation group.
 	 */
 	rval = 0;
-	while (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft) && agno < mp->m_sb.sb_agcount) {
-		cond_resched();
+	while (agno < mp->m_sb.sb_agcount) {
+		struct xfs_inobt_rec_incore	*irbp = irbuf;
+		struct xfs_inobt_rec_incore	*irbufend = irbuf + nirbuf;
+		bool				end_of_ag = false;
+		int				icount = 0;
+		int				stat;
+
 		error = xfs_ialloc_read_agi(mp, NULL, agno, &agbp);
 		if (error)
 			break;
@@ -407,10 +405,6 @@
 		 */
 		cur = xfs_inobt_init_cursor(mp, NULL, agbp, agno,
 					    XFS_BTNUM_INO);
-		irbp = irbuf;
-		irbufend = irbuf + nirbuf;
-		end_of_ag = 0;
-		icount = 0;
 		if (agino > 0) {
 			/*
 			 * In the middle of an allocation group, we need to get
@@ -435,7 +429,7 @@
 			error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &stat);
 		}
 		if (error || stat == 0) {
-			end_of_ag = 1;
+			end_of_ag = true;
 			goto del_cursor;
 		}
  
@@ -448,7 +442,7 @@
  
 			error = xfs_inobt_get_rec(cur, &r, &stat);
 			if (error || stat == 0) {
-				end_of_ag = 1;
+				end_of_ag = true;
 				goto del_cursor;
 			}
  
@@ -470,7 +464,7 @@
 			agino = r.ir_startino + XFS_INODES_PER_CHUNK;
 			error = xfs_btree_increment(cur, 0, &stat);
 			if (error || stat == 0) {
-				end_of_ag = 1;
+				end_of_ag = true;
 				goto del_cursor;
 			}
 			cond_resched();
@@ -491,7 +485,7 @@
 		 */
 		irbufend = irbp;
 		for (irbp = irbuf;
-		     irbp < irbufend && XFS_BULKSTAT_UBLEFT(ac.ac_ubleft);
+		     irbp < irbufend && ac.ac_ubleft >= statstruct_size;
 		     irbp++) {
 			error = xfs_bulkstat_ag_ichunk(mp, agno, irbp,
 					formatter, statstruct_size, &ac,
  
@@ -502,17 +496,15 @@
 			cond_resched();
 		}
  
-		/*
-		 * Set up for the next loop iteration.
-		 */
-		if (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft)) {
-			if (end_of_ag) {
-				agno++;
-				agino = 0;
-			} else
-				agino = XFS_INO_TO_AGINO(mp, lastino);
-		} else
+		/* If we've run out of space, we are done */
+		if (ac.ac_ubleft < statstruct_size)
 			break;
+
+		if (end_of_ag) {
+			agno++;
+			agino = 0;
+		} else
+			agino = XFS_INO_TO_AGINO(mp, lastino);
 	}
 	/*
 	 * Done, we're either out of filesystem or space to put the data.
...	...	@@ -348,30 +348,23 @@
348	348	xfs_agino_t agino; /* inode # in allocation group */
349	349	xfs_agnumber_t agno; /* allocation group number */
350	350	xfs_btree_cur_t cur; / btree cursor for ialloc btree */
351		- int end_of_ag; /* set if we've seen the ag end */
352		- int error; /* error code */
353		- int icount; /* count of inodes good in irbuf */
354	351	size_t irbsize; /* size of irec buffer in bytes */
355		- xfs_ino_t ino; /* inode number (filesystem) */
356		- xfs_inobt_rec_incore_t irbp; / current irec buffer pointer */
357	352	xfs_inobt_rec_incore_t irbuf; / start of irec buffer */
358		- xfs_inobt_rec_incore_t irbufend; / end of good irec buffer entries */
359	353	xfs_ino_t lastino; /* last inode number returned */
360	354	int nirbuf; /* size of irbuf */
361	355	int rval; /* return value error code */
362	356	int ubcount; /* size of user's buffer */
363		- int stat;
364	357	struct xfs_bulkstat_agichunk ac;
	358	+ int error = 0;
365	359
366	360	/*
367	361	* Get the last inode value, see if there's nothing to do.
368	362	*/
369		- ino = (xfs_ino_t)*lastinop;
370		- lastino = ino;
371		- agno = XFS_INO_TO_AGNO(mp, ino);
372		- agino = XFS_INO_TO_AGINO(mp, ino);
	363	+ lastino = *lastinop;
	364	+ agno = XFS_INO_TO_AGNO(mp, lastino);
	365	+ agino = XFS_INO_TO_AGINO(mp, lastino);
373	366	if (agno >= mp->m_sb.sb_agcount \|\|
374		- ino != XFS_AGINO_TO_INO(mp, agno, agino)) {
	367	+ lastino != XFS_AGINO_TO_INO(mp, agno, agino)) {
375	368	*done = 1;
376	369	*ubcountp = 0;
377	370	return 0;
...	...	@@ -396,8 +389,13 @@
396	389	* inode returned; 0 means start of the allocation group.
397	390	*/
398	391	rval = 0;
399		- while (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft) && agno < mp->m_sb.sb_agcount) {
400		- cond_resched();
	392	+ while (agno < mp->m_sb.sb_agcount) {
	393	+ struct xfs_inobt_rec_incore *irbp = irbuf;
	394	+ struct xfs_inobt_rec_incore *irbufend = irbuf + nirbuf;
	395	+ bool end_of_ag = false;
	396	+ int icount = 0;
	397	+ int stat;
	398	+
401	399	error = xfs_ialloc_read_agi(mp, NULL, agno, &agbp);
402	400	if (error)
403	401	break;
...	...	@@ -407,10 +405,6 @@
407	405	*/
408	406	cur = xfs_inobt_init_cursor(mp, NULL, agbp, agno,
409	407	XFS_BTNUM_INO);
410		- irbp = irbuf;
411		- irbufend = irbuf + nirbuf;
412		- end_of_ag = 0;
413		- icount = 0;
414	408	if (agino > 0) {
415	409	/*
416	410	* In the middle of an allocation group, we need to get
...	...	@@ -435,7 +429,7 @@
435	429	error = xfs_inobt_lookup(cur, 0, XFS_LOOKUP_GE, &stat);
436	430	}
437	431	if (error \|\| stat == 0) {
438		- end_of_ag = 1;
	432	+ end_of_ag = true;
439	433	goto del_cursor;
440	434	}
441	435
...	...	@@ -448,7 +442,7 @@
448	442
449	443	error = xfs_inobt_get_rec(cur, &r, &stat);
450	444	if (error \|\| stat == 0) {
451		- end_of_ag = 1;
	445	+ end_of_ag = true;
452	446	goto del_cursor;
453	447	}
454	448
...	...	@@ -470,7 +464,7 @@
470	464	agino = r.ir_startino + XFS_INODES_PER_CHUNK;
471	465	error = xfs_btree_increment(cur, 0, &stat);
472	466	if (error \|\| stat == 0) {
473		- end_of_ag = 1;
	467	+ end_of_ag = true;
474	468	goto del_cursor;
475	469	}
476	470	cond_resched();
...	...	@@ -491,7 +485,7 @@
491	485	*/
492	486	irbufend = irbp;
493	487	for (irbp = irbuf;
494		- irbp < irbufend && XFS_BULKSTAT_UBLEFT(ac.ac_ubleft);
	488	+ irbp < irbufend && ac.ac_ubleft >= statstruct_size;
495	489	irbp++) {
496	490	error = xfs_bulkstat_ag_ichunk(mp, agno, irbp,
497	491	formatter, statstruct_size, &ac,
498	492
...	...	@@ -502,17 +496,15 @@
502	496	cond_resched();
503	497	}
504	498
505		- /*
506		- * Set up for the next loop iteration.
507		- */
508		- if (XFS_BULKSTAT_UBLEFT(ac.ac_ubleft)) {
509		- if (end_of_ag) {
510		- agno++;
511		- agino = 0;
512		- } else
513		- agino = XFS_INO_TO_AGINO(mp, lastino);
514		- } else
	499	+ /* If we've run out of space, we are done */
	500	+ if (ac.ac_ubleft < statstruct_size)
515	501	break;
	502	+
	503	+ if (end_of_ag) {
	504	+ agno++;
	505	+ agino = 0;
	506	+ } else
	507	+ agino = XFS_INO_TO_AGINO(mp, lastino);
516	508	}
517	509	/*
518	510	* Done, we're either out of filesystem or space to put the data.