[PATCH]fix fbno in xfs_dir2_node_addname_int

[PATCH]fix fbno in xfs_dir2_node_addname_int

[Kevin Xu]

[-- Attachment #1: Type: text/plain, Size: 148 bytes --]

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.

[-- Attachment #2: usig-xfs-fix-080322.patch --]
[-- Type: text/x-patch, Size: 379 bytes --]

--- linux-2.6-xfs/fs/xfs/xfs_dir2_node.c	2008-03-22 22:41:12.118699220 +0800
+++ linux-xfs-usig/fs/xfs/xfs_dir2_node.c	2008-03-22 22:48:07.694781678 +0800
@@ -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;
+				}
 			}
 		}
 	}

Re: [PATCH]fix fbno in xfs_dir2_node_addname_int

[David Chinner]

On Sun, Mar 23, 2008 at 08:51:14AM +0800, Kevin Xu wrote:
> 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.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH]fix fbno in xfs_dir2_node_addname_int

[Eric Sandeen]

Kevin Xu wrote:
> 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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs