Skip to contentSkip to conversation

fix fbno in xfs_dir2_node_addname_int

PATCH In review Kevin Xu 3 participants 3 messages Mar 23, 2008 – Apr 13, 2009 XFS filesystem

lore
Share & tools
lore

Kevin Xu

author
#0 lore raw

PATCH 1/1 fix fbno in xfs_dir2_node_addname_int

if we didn't find a freespace block for our new entry in the current freeindex block, return to the first freeindex block and continue to check.

fs/xfs/xfs_dir2_node.c +3 -1
@@ -1502,8 +1502,10 @@ xfs_dir2_node_addname_int(
 				 */
 				xfs_da_brelse(tp, fbp);
 				fbp = NULL;
-				if (fblk && fblk->bp)
+				if (fblk && fblk->bp) {
 					fblk->bp = NULL;
+					fbno = -1;
+				}
 			}
 		}
 	}
#1 lore raw
Quoted 3 lines · Kevin Xu

if we didn't find a freespace block for our new entry in the current freeindex block,
return to the first freeindex block and continue to check.

What is the test case that demonstrates this problem?

Looking at the impact of setting fbno = -1 if we don't find a suitable free space, the next iteration of the loop will do:

1454 if (fbp == NULL) {
1455 /*

   1456                          * Happens the first time through unless lookup gave
   1457                          * us a freespace block to start with.
   1458                          */

1459 if (++fbno == 0)
1460 fbno = XFS_DIR2_FREE_FIRSTDB(mp);

and

define XFS_DIR2_FREE_FIRSTDB(mp)       \
        xfs_dir2_byte_to_db(mp, XFS_DIR2_FREE_OFFSET)

Is a fixed offset into the directory. Hence resetting fbno = -1 will force us to look up the same freespace block on every loop iteration. That looks like it will livelock as soon as the first free space block does not have enough space for the desired entry......

Cheers,

Dave.

Signature · 3 lines
Dave Chinner
Principal Engineer
SGI Australian Software Group
#2 lore raw
In reply to Kevin Xu
Quoted 4 lines · Kevin Xu

if we didn't find a freespace block for our new entry in the current freeindex block,
return to the first freeindex block and continue to check.

Kevin, you sent this and another patch a while ago, and we had asked for more information in the changelogs, a Signed-off-by: line, and a testcase if at all possible.

Any chance that you can provide these things?

Thanks,
-Eric

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