odd code in xfs_remove
unlike xfs_rmdir or xfs_rename xfs_remove always passes 0 instead of the reserved blocks to xfs_dir_removename which means the latter always operated in the ENOSPC mode. This seems rather strange to me, so a second pair of eyes, especially on a tree with history back to day 0 would be nice.
Quoted 5 lines · Christoph Hellwig
unlike
xfs_rmdirorxfs_renamexfs_removealways passes 0 instead of the reserved blocks toxfs_dir_removenamewhich means the latter always operated in the ENOSPC mode. This seems rather strange to me, so a second pair of eyes, especially on a tree with history back to day 0 would be nice.
Originially, back in the days of dir1 (1994) when xfs_dir_removename() was expanded to take a block count, only xfs_rename() passed a value in (MAX_EXT_NEEDED). Both xfs_rmdir and xfs_remove passed zero.
Then some time later (pv 519072):
irix/kern/fs/xfs/xfs_vnodeops.c - 1.356
- Add more vnode tracing points. Remove some cruft from merge errors. Speed up xfs_remove by getting a real space reservation unless the fs is full, then reverting to a zero space reservation (bug 519072).
The speed up was for dir v1, which doesn't exist on linux. Then in pv 653987:
irix/kern/fs/xfs/xfs_vnodeops.c - 1.395
- In xfs_link, call xfs_dir_canenter before xfs_dir_createname if we have no space reservation.
In xfs_rmdir, get a 0 space reservation if we can't get the real space reservation (as is done for xfs_remove).
In xfs_symlink, properly account for resblks throughout so we don't make it go negative to a xfs_bmapi call.
The key point here is that in rev 1.395, the variable resblks got passed into xfs_dir_removename(), whilst in 1.356, it didn't. This was work that preceeded dir v2, so was probably fixing bugs found as a result.
The remove name case with resblks = 0 avoids block allocation during modification of the alloc btree to insert newly freed blocks at ENOSPC; instead it just leaves the empty leaf block in place in the directory.
Looks like no harm is done by this and it's a rarely hit corner case, but it would appear that we should be passing in resblks in remove if only to avoid a potential transaction reservation overrun....
Cheers,
Dave.
Signature · 3 lines
Dave Chinner Principal Engineer SGI Australian Software Group
Quoted 4 lines · David Chinner
Looks like no harm is done by this and it's a rarely hit corner case, but it would appear that we should be passing in resblks in remove if only to avoid a potential transaction reservation overrun....
Okay, I'll prepare a patch and test it a little. This was the only spurious difference between xfs_remove and xfs_rmdir left, after this we can almost trivially merge them.