Hi Christoph, hi all!
When I move a directory into another on a tmpfs filesystem, the
link-count of the new parent directory isn't getting incremented.
(which leads to find getting hickup, which eg. lets dpkg produce a
"bzip2" binary package without binaries, generally not a nice thing)
test:
isax@home:/tmp/test$ mkdir t1
isax@home:/tmp/test$ mkdir t2
isax@home:/tmp/test$ mv t2 t1
isax@home:/tmp/test$ ls -lRa
.:
total 0
drwxr-xr-x 5 isax isax 0 Feb 13 23:05 .
drwxrwxrwt 13 root root 0 Feb 13 23:04 ..
drwxr-xr-x 2 isax isax 0 Feb 13 23:05 t1
./t1:
total 0
drwxr-xr-x 2 isax isax 0 Feb 13 23:05 .
drwxr-xr-x 5 isax isax 0 Feb 13 23:05 ..
drwxr-xr-x 2 isax isax 0 Feb 13 23:05 t2
./t1/t2:
total 0
drwxr-xr-x 2 isax isax 0 Feb 13 23:05 .
drwxr-xr-x 2 isax isax 0 Feb 13 23:05 ..
the link count of "t1", "t1/." and "t1/t2/.." should be "3", not "2".
The following patch seems to work fine for me and is tested on my
machine running debian's 2.4.17-1 and user-mode-linux 2.5.1-1.
This is my first patch to the kernel, so I suppose there is a really
huge mistake in my patch I don't see now...
--- linux/mm/shmem.orig Wed Feb 13 00:56:14 2002
+++ linux/mm/shmem.c Wed Feb 13 18:09:04 2002
@@ -1085,6 +1085,9 @@
{
int error = -ENOTEMPTY;
+ if (S_ISDIR(old_dentry->d_inode->i_mode)) {
+ new_dir->i_nlink++;
+ }
if (shmem_empty(new_dentry)) {
struct inode *inode = new_dentry->d_inode;
if (inode) {
--
uli martens
On Thu, Feb 14, 2002 at 02:07:18AM +0100, Uli Martens wrote:
> When I move a directory into another on a tmpfs filesystem, the
> link-count of the new parent directory isn't getting incremented.
> (which leads to find getting hickup, which eg. lets dpkg produce a
> "bzip2" binary package without binaries, generally not a nice thing)
Yeah, that optimization actually breaks on so many filesystems
(AFS/Coda/iso9660/msdos/any filesystem which contains more than 65538
subdirectories in a directory/you name it) that they even provided a
flag to disable it (-noleaf), which in my opinion should always be the
default setting.
Another trick on the 'filesystem side' of things is to set the directory
linkcount to 1. Basically find always subtracts 2 and then as long as
the count is not zero, it will stat the entry to see if it's a
directory. So a directory linkcount of 1 will 'underflow' and find will
work correctly for the first 65535 sub-directories.
It's been on my todo list to use this fix in Coda as we have some link
count issues when we're flipping entries between 'faked' symlinks for
mountpoints/conflicts and real directories.
Jan
> @@ -1085,6 +1085,9 @@
> {
> int error = -ENOTEMPTY;
>
> + if (S_ISDIR(old_dentry->d_inode->i_mode)) {
> + new_dir->i_nlink++;
> + }
> if (shmem_empty(new_dentry)) {
> struct inode *inode = new_dentry->d_inode;
> if (inode) {
ps. shouldn't the linkcount of the old_dir get decremented too? Also you
should only change the linkcounts when the operation completed
successfully. i.e. something more like,
--- /tmp/shmem.c.orig Thu Feb 14 01:17:23 2002
+++ /tmp/shmem.c Thu Feb 14 01:18:25 2002
@@ -1100,6 +1100,10 @@
error = 0;
old_dentry->d_inode->i_ctime = old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME;
}
+ if (!error && S_ISDIR(old_dentry->d_inode->i_mode)) {
+ old_dir->i_nlink--;
+ new_dir->i_nlink++;
+ }
return error;
}
On Thu, 2002-02-14 at 07:19, Jan Harkes wrote:
>
> ps. shouldn't the linkcount of the old_dir get decremented too? Also you
> should only change the linkcounts when the operation completed
> successfully. i.e. something more like,
Oops, you're right, the linkcount of old_dir isn't decremented at the
moment, too. I will test your patch this evening, but I think it looks
better than mine... ;)
> --- /tmp/shmem.c.orig Thu Feb 14 01:17:23 2002
> +++ /tmp/shmem.c Thu Feb 14 01:18:25 2002
> @@ -1100,6 +1100,10 @@
> error = 0;
> old_dentry->d_inode->i_ctime = old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME;
> }
> + if (!error && S_ISDIR(old_dentry->d_inode->i_mode)) {
> + old_dir->i_nlink--;
> + new_dir->i_nlink++;
> + }
> return error;
> }
--
uli martens
Hi Uli,
On 14 Feb 2002, Uli Martens wrote:
> Oops, you're right, the linkcount of old_dir isn't decremented at
> the moment, too. I will test your patch this evening, but I think it
> looks better than mine... ;)
I prefer this one.
Linus, Marcelo, please apply.
The patch fixes the refcounting of directories on renames in tmpfs.
Greetings
Christoph
diff -uNr 18-rc1/mm/shmem.c c/mm/shmem.c
--- 18-rc1/mm/shmem.c Thu Jan 17 10:06:05 2002
+++ c/mm/shmem.c Tue Feb 19 17:31:23 2002
@@ -1083,19 +1083,25 @@
*/
static int shmem_rename(struct inode * old_dir, struct dentry *old_dentry, struct inode * new_dir,struct dentry *new_dentry)
{
- int error = -ENOTEMPTY;
+ struct inode *inode;
- if (shmem_empty(new_dentry)) {
- struct inode *inode = new_dentry->d_inode;
- if (inode) {
- inode->i_ctime = CURRENT_TIME;
- inode->i_nlink--;
- dput(new_dentry);
- }
- error = 0;
- old_dentry->d_inode->i_ctime = old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME;
+ if (!shmem_empty(new_dentry))
+ return -ENOTEMPTY;
+
+ inode = new_dentry->d_inode;
+ if (inode) {
+ inode->i_ctime = CURRENT_TIME;
+ inode->i_nlink--;
+ dput(new_dentry);
+ }
+ inode = old_dentry->d_inode;
+ if (S_ISDIR(inode->i_mode)) {
+ old_dir->i_nlink--;
+ new_dir->i_nlink++;
}
- return error;
+
+ inode->i_ctime = old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME;
+ return 0;
}
static int shmem_symlink(struct inode * dir, struct dentry *dentry, const char * symname)