[PATCH]: reiserfs: D-clear-i_blocks.patch

[PATCH]: reiserfs: D-clear-i_blocks.patch

[Nikita Danilov]

Hello, Linus,

This patch sets inode.i_blocks to zero on deletion of reiserfs
file. This in particular cures hard to believe bug when saving file in
EMACS caused top to loose sight of all processes:
 . reiserfs didn't properly cleared i_blocks when removing
   symlinks. Actually -7 was inserted into unsigned i_blocks field. This
   didn't usually hurt because file is being deleted;
 . inode is reused for procfs and neither get_new_inode() nor
   proc_read_inode() cleared i_blocks;
 . now procfs inode has huge i_blocks field;
 . top calls stat on it and libc wrapper returns EOVERFLOW, as i_blocks
   doesn't fit into user-level struct.
 . top sees nothing.
  
[lkml: please CC me, I am not subscribed.]

Nikita.
diff -rup linux-2.4.8-pre3/fs/reiserfs/inode.c linux-2.4.8-pre3.patched/fs/reiserfs/inode.c
--- linux-2.4.8-pre3/fs/reiserfs/inode.c	Wed Aug  1 17:21:10 2001
+++ linux-2.4.8-pre3.patched/fs/reiserfs/inode.c	Wed Aug  1 21:33:37 2001
@@ -55,6 +55,7 @@ void reiserfs_delete_inode (struct inode
 	;
     }
     clear_inode (inode); /* note this must go after the journal_end to prevent deadlock */
+    inode->i_blocks = 0;
     unlock_kernel() ;
 }
 

Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[Andreas Dilger]

Nikita writes:
> This patch sets inode.i_blocks to zero on deletion of reiserfs
> file. This in particular cures hard to believe bug when saving file in
> EMACS caused top to loose sight of all processes:
>  . reiserfs didn't properly cleared i_blocks when removing
>    symlinks. Actually -7 was inserted into unsigned i_blocks field. This
>    didn't usually hurt because file is being deleted;
>  . inode is reused for procfs and neither get_new_inode() nor
>    proc_read_inode() cleared i_blocks;
>  . now procfs inode has huge i_blocks field;
>  . top calls stat on it and libc wrapper returns EOVERFLOW, as i_blocks
>    doesn't fit into user-level struct.
>  . top sees nothing.

I wonder if you don't have a different problem hiding in there somewhere.
I was thinking that if a non-zero i_blocks field was causing problems for
reiserfs, maybe clear_inode() should zero this out for everyone.

Looking through the reiserfs code (which is rather hard to follow), I see
that reiserfs_cut_from_item() and prepare_for_delete_or_cut() are changing
i_blocks, and this is (eventually) called from reiserfs_delete_item() and
reiserfs_delete_object().  Not that I can point to any specific code, but
to me it would seem that you are not counting i_blocks correctly somewhere,
and eventually decrement i_blocks too much (hence -7 in i_blocks).

So this makes me believe that setting i_blocks=0 is just hiding a block
leak somewhere in the reiserfs code, or math errors somewhere.

It is also true that clear_inode() or get_new_inode() (or proc_read_inode())
should initialize all of the used fields to zero so that a problem in one
filesystem can't cause problems elsewhere.  It appears that i_blocks is
set to zero by ext2_new_inode(), so probably proc_read_inode() should do
the same (since it uses i_blocks).

Cheers, Andreas

PS - could someone on the reiserfs team (or Linus) run the reiserfs code
     through "indent" (or auto format in emacs) per Documentation/CodingStyle?
     It is really a gross mess, to such a point that you can hardly see what
     is going on.  It's not just that it is a different indent style, it has
     no coherent indentation or comment formatting at all.  Maybe for 2.5?
-- 
Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[Alexander Viro]

On Thu, 2 Aug 2001, Nikita Danilov wrote:

> Hello, Linus,
> 
> This patch sets inode.i_blocks to zero on deletion of reiserfs
> file. This in particular cures hard to believe bug when saving file in
> EMACS caused top to loose sight of all processes:
>  . reiserfs didn't properly cleared i_blocks when removing
>    symlinks. Actually -7 was inserted into unsigned i_blocks field. This
>    didn't usually hurt because file is being deleted;
>  . inode is reused for procfs and neither get_new_inode() nor
>    proc_read_inode() cleared i_blocks;
>  . now procfs inode has huge i_blocks field;
>  . top calls stat on it and libc wrapper returns EOVERFLOW, as i_blocks
>    doesn't fit into user-level struct.
>  . top sees nothing.
>   
> [lkml: please CC me, I am not subscribed.]

Thanks for spotting, but I have to disagree with analysis.
	a) it's a procfs bug. We leave ->i_blocks uninitialized and inode
constructor leaves it in undefined state.
	b) I'd rather fix it once in fs/inode.c::clean_inode() instead of
hunting similar bugs down again and again.

See if the patch below works for you. It makes sure that inodes passed to
->read_inode() or returned by new_inode() have zero in i_blocks and removes
the redundant assignments in filesystems. Warning: it's completely untested.

diff -urN S8-pre3/drivers/isdn/avmb1/capifs.c S8-pre3-i_blocks/drivers/isdn/avmb1/capifs.c
--- S8-pre3/drivers/isdn/avmb1/capifs.c	Tue Jul  3 21:09:09 2001
+++ S8-pre3-i_blocks/drivers/isdn/avmb1/capifs.c	Thu Aug  2 13:30:28 2001
@@ -389,7 +389,6 @@
 	struct inode *inode = new_inode(sb);
 	if (inode) {
 		inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-		inode->i_blocks = 0;
 		inode->i_blksize = 1024;
 		inode->i_uid = inode->i_gid = 0;
 	}
diff -urN S8-pre3/fs/autofs/inode.c S8-pre3-i_blocks/fs/autofs/inode.c
--- S8-pre3/fs/autofs/inode.c	Tue Jul  3 21:09:13 2001
+++ S8-pre3-i_blocks/fs/autofs/inode.c	Thu Aug  2 13:30:47 2001
@@ -212,7 +212,6 @@
 	inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO;
 	inode->i_nlink = 2;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_blocks = 0;
 	inode->i_blksize = 1024;
 
 	if ( ino == AUTOFS_ROOT_INO ) {
diff -urN S8-pre3/fs/autofs4/inode.c S8-pre3-i_blocks/fs/autofs4/inode.c
--- S8-pre3/fs/autofs4/inode.c	Fri Feb 16 22:52:15 2001
+++ S8-pre3-i_blocks/fs/autofs4/inode.c	Thu Aug  2 13:30:51 2001
@@ -307,7 +307,6 @@
 		inode->i_gid = 0;
 	}
 	inode->i_blksize = PAGE_CACHE_SIZE;
-	inode->i_blocks = 0;
 	inode->i_rdev = 0;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
 
diff -urN S8-pre3/fs/bfs/dir.c S8-pre3-i_blocks/fs/bfs/dir.c
--- S8-pre3/fs/bfs/dir.c	Fri Feb 16 20:12:01 2001
+++ S8-pre3-i_blocks/fs/bfs/dir.c	Thu Aug  2 13:31:10 2001
@@ -93,7 +93,7 @@
 	inode->i_uid = current->fsuid;
 	inode->i_gid = (dir->i_mode & S_ISGID) ? dir->i_gid : current->fsgid;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	inode->i_op = &bfs_file_inops;
 	inode->i_fop = &bfs_file_operations;
 	inode->i_mapping->a_ops = &bfs_aops;
diff -urN S8-pre3/fs/devfs/base.c S8-pre3-i_blocks/fs/devfs/base.c
--- S8-pre3/fs/devfs/base.c	Sun Jul 29 01:54:47 2001
+++ S8-pre3-i_blocks/fs/devfs/base.c	Thu Aug  2 13:31:31 2001
@@ -2263,7 +2263,6 @@
 	printk ("%s: read_inode(%d): VFS inode: %p  devfs_entry: %p\n",
 		DEVFS_NAME, (int) inode->i_ino, inode, de);
 #endif
-    inode->i_blocks = 0;
     inode->i_blksize = 1024;
     inode->i_op = &devfs_iops;
     inode->i_fop = &devfs_fops;
diff -urN S8-pre3/fs/devpts/inode.c S8-pre3-i_blocks/fs/devpts/inode.c
--- S8-pre3/fs/devpts/inode.c	Wed Apr 18 00:35:58 2001
+++ S8-pre3-i_blocks/fs/devpts/inode.c	Thu Aug  2 13:31:40 2001
@@ -145,7 +145,6 @@
 		goto fail_free;
 	inode->i_ino = 1;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_blocks = 0;
 	inode->i_blksize = 1024;
 	inode->i_uid = inode->i_gid = 0;
 	inode->i_mode = S_IFDIR | S_IRUGO | S_IXUGO | S_IWUSR;
@@ -193,7 +192,6 @@
 	if (!inode)
 		return;
 	inode->i_ino = number+2;
-	inode->i_blocks = 0;
 	inode->i_blksize = 1024;
 	inode->i_uid = sbi->setuid ? sbi->uid : current->fsuid;
 	inode->i_gid = sbi->setgid ? sbi->gid : current->fsgid;
diff -urN S8-pre3/fs/efs/inode.c S8-pre3-i_blocks/fs/efs/inode.c
--- S8-pre3/fs/efs/inode.c	Fri Feb 16 13:07:52 2001
+++ S8-pre3-i_blocks/fs/efs/inode.c	Thu Aug  2 13:32:14 2001
@@ -93,11 +93,8 @@
 	inode->i_ctime = be32_to_cpu(efs_inode->di_ctime);
 
 	/* this is the number of blocks in the file */
-	if (inode->i_size == 0) {
-		inode->i_blocks = 0;
-	} else {
+	if (inode->i_size)
 		inode->i_blocks = ((inode->i_size - 1) >> EFS_BLOCKSIZE_BITS) + 1;
-	}
 
 	/*
 	 * BUG: irix dev_t is 32-bits. linux dev_t is only 16-bits.
diff -urN S8-pre3/fs/ext2/ialloc.c S8-pre3-i_blocks/fs/ext2/ialloc.c
--- S8-pre3/fs/ext2/ialloc.c	Tue Jul  3 21:09:13 2001
+++ S8-pre3-i_blocks/fs/ext2/ialloc.c	Thu Aug  2 13:32:24 2001
@@ -431,7 +431,6 @@
 
 	inode->i_ino = j;
 	inode->i_blksize = PAGE_SIZE;	/* This is the optimal IO size (for stat), not the fs block size */
-	inode->i_blocks = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	inode->u.ext2_i.i_new_inode = 1;
 	inode->u.ext2_i.i_flags = dir->u.ext2_i.i_flags;
diff -urN S8-pre3/fs/inode.c S8-pre3-i_blocks/fs/inode.c
--- S8-pre3/fs/inode.c	Sun Jul 29 01:54:47 2001
+++ S8-pre3-i_blocks/fs/inode.c	Thu Aug  2 13:38:23 2001
@@ -750,6 +750,7 @@
 	inode->i_nlink = 1;
 	atomic_set(&inode->i_writecount, 0);
 	inode->i_size = 0;
+	inode->i_blocks = 0;
 	inode->i_generation = 0;
 	memset(&inode->i_dquot, 0, sizeof(inode->i_dquot));
 	inode->i_pipe = NULL;
diff -urN S8-pre3/fs/isofs/inode.c S8-pre3-i_blocks/fs/isofs/inode.c
--- S8-pre3/fs/isofs/inode.c	Thu Apr 19 23:46:45 2001
+++ S8-pre3-i_blocks/fs/isofs/inode.c	Thu Aug  2 13:33:16 2001
@@ -1173,7 +1173,7 @@
 	}
 	inode->i_uid = inode->i_sb->u.isofs_sb.s_uid;
 	inode->i_gid = inode->i_sb->u.isofs_sb.s_gid;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 
 
 	inode->u.isofs_i.i_section_size = isonum_733 (de->size);
diff -urN S8-pre3/fs/minix/bitmap.c S8-pre3-i_blocks/fs/minix/bitmap.c
--- S8-pre3/fs/minix/bitmap.c	Fri Feb 16 20:12:05 2001
+++ S8-pre3-i_blocks/fs/minix/bitmap.c	Thu Aug  2 13:33:50 2001
@@ -261,7 +261,7 @@
 	inode->i_gid = (dir->i_mode & S_ISGID) ? dir->i_gid : current->fsgid;
 	inode->i_ino = j;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	insert_inode_hash(inode);
 	mark_inode_dirty(inode);
 
diff -urN S8-pre3/fs/minix/inode.c S8-pre3-i_blocks/fs/minix/inode.c
--- S8-pre3/fs/minix/inode.c	Thu Apr 19 23:46:46 2001
+++ S8-pre3-i_blocks/fs/minix/inode.c	Thu Aug  2 13:34:06 2001
@@ -456,7 +456,7 @@
 	inode->i_nlink = raw_inode->i_nlinks;
 	inode->i_size = raw_inode->i_size;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = raw_inode->i_time;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	for (block = 0; block < 9; block++)
 		inode->u.minix_i.u.i1_data[block] = raw_inode->i_zone[block];
 	if (S_ISREG(inode->i_mode)) {
@@ -509,7 +509,7 @@
 	inode->i_mtime = raw_inode->i_mtime;
 	inode->i_atime = raw_inode->i_atime;
 	inode->i_ctime = raw_inode->i_ctime;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	for (block = 0; block < 10; block++)
 		inode->u.minix_i.u.i2_data[block] = raw_inode->i_zone[block];
 	if (S_ISREG(inode->i_mode)) {
diff -urN S8-pre3/fs/ramfs/inode.c S8-pre3-i_blocks/fs/ramfs/inode.c
--- S8-pre3/fs/ramfs/inode.c	Thu May 24 18:26:54 2001
+++ S8-pre3-i_blocks/fs/ramfs/inode.c	Thu Aug  2 13:34:25 2001
@@ -117,7 +117,6 @@
 		inode->i_uid = current->fsuid;
 		inode->i_gid = current->fsgid;
 		inode->i_blksize = PAGE_CACHE_SIZE;
-		inode->i_blocks = 0;
 		inode->i_rdev = NODEV;
 		inode->i_mapping->a_ops = &ramfs_aops;
 		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
diff -urN S8-pre3/fs/reiserfs/inode.c S8-pre3-i_blocks/fs/reiserfs/inode.c
--- S8-pre3/fs/reiserfs/inode.c	Sun Jul 29 01:54:47 2001
+++ S8-pre3-i_blocks/fs/reiserfs/inode.c	Thu Aug  2 13:34:54 2001
@@ -940,7 +940,6 @@
 	inode->i_op = &page_symlink_inode_operations;
 	inode->i_mapping->a_ops = &reiserfs_address_space_operations;
     } else {
-	inode->i_blocks = 0;
 	init_special_inode(inode, inode->i_mode, rdev) ;
     }
 }
diff -urN S8-pre3/fs/sysv/ialloc.c S8-pre3-i_blocks/fs/sysv/ialloc.c
--- S8-pre3/fs/sysv/ialloc.c	Sun Jul 29 01:54:48 2001
+++ S8-pre3-i_blocks/fs/sysv/ialloc.c	Thu Aug  2 13:36:08 2001
@@ -164,7 +164,7 @@
 	inode->i_uid = current->fsuid;
 	inode->i_ino = fs16_to_cpu(sb, ino);
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	insert_inode_hash(inode);
 	mark_inode_dirty(inode);
 
diff -urN S8-pre3/fs/sysv/inode.c S8-pre3-i_blocks/fs/sysv/inode.c
--- S8-pre3/fs/sysv/inode.c	Sun Jul 29 01:54:48 2001
+++ S8-pre3-i_blocks/fs/sysv/inode.c	Thu Aug  2 13:36:13 2001
@@ -171,7 +171,7 @@
 	inode->i_atime = fs32_to_cpu(sb, raw_inode->i_atime);
 	inode->i_mtime = fs32_to_cpu(sb, raw_inode->i_mtime);
 	inode->i_ctime = fs32_to_cpu(sb, raw_inode->i_ctime);
-	inode->i_blocks = inode->i_blksize = 0;
+	inode->i_blksize = 0;
 	for (block = 0; block < 10+1+1+1; block++)
 		read3byte(sb, &raw_inode->i_a.i_addb[3*block],
 			(unsigned char*)&inode->u.sysv_i.i_data[block]);
diff -urN S8-pre3/fs/udf/ialloc.c S8-pre3-i_blocks/fs/udf/ialloc.c
--- S8-pre3/fs/udf/ialloc.c	Tue Jul  3 21:09:13 2001
+++ S8-pre3-i_blocks/fs/udf/ialloc.c	Thu Aug  2 13:36:22 2001
@@ -128,7 +128,6 @@
 	UDF_I_LOCATION(inode).partitionReferenceNum = UDF_I_LOCATION(dir).partitionReferenceNum;
 	inode->i_ino = udf_get_lb_pblock(sb, UDF_I_LOCATION(inode), 0);
 	inode->i_blksize = PAGE_SIZE;
-	inode->i_blocks = 0;
 	UDF_I_LENEATTR(inode) = 0;
 	UDF_I_LENALLOC(inode) = 0;
 	if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_EXTENDED_FE))
diff -urN S8-pre3/fs/ufs/ialloc.c S8-pre3-i_blocks/fs/ufs/ialloc.c
--- S8-pre3/fs/ufs/ialloc.c	Fri Feb 16 20:12:10 2001
+++ S8-pre3-i_blocks/fs/ufs/ialloc.c	Thu Aug  2 13:37:05 2001
@@ -268,7 +268,6 @@
 
 	inode->i_ino = cg * uspi->s_ipg + bit;
 	inode->i_blksize = PAGE_SIZE;	/* This is the optimal IO size (for stat), not the fs block size */
-	inode->i_blocks = 0;
 	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME;
 	inode->u.ufs_i.i_flags = dir->u.ufs_i.i_flags;
 	inode->u.ufs_i.i_lastfrag = 0;
diff -urN S8-pre3/mm/shmem.c S8-pre3-i_blocks/mm/shmem.c
--- S8-pre3/mm/shmem.c	Sun Jul 29 01:54:48 2001
+++ S8-pre3-i_blocks/mm/shmem.c	Thu Aug  2 13:37:35 2001
@@ -508,7 +508,6 @@
 		inode->i_uid = current->fsuid;
 		inode->i_gid = current->fsgid;
 		inode->i_blksize = PAGE_CACHE_SIZE;
-		inode->i_blocks = 0;
 		inode->i_rdev = NODEV;
 		inode->i_mapping->a_ops = &shmem_aops;
 		inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;

Re: [reiserfs-dev] Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[Nikita Danilov]

Andreas Dilger writes:
 > Nikita writes:
 > > This patch sets inode.i_blocks to zero on deletion of reiserfs
 > > file. This in particular cures hard to believe bug when saving file in
 > > EMACS caused top to loose sight of all processes:
 > >  . reiserfs didn't properly cleared i_blocks when removing
 > >    symlinks. Actually -7 was inserted into unsigned i_blocks field. This
 > >    didn't usually hurt because file is being deleted;
 > >  . inode is reused for procfs and neither get_new_inode() nor
 > >    proc_read_inode() cleared i_blocks;
 > >  . now procfs inode has huge i_blocks field;
 > >  . top calls stat on it and libc wrapper returns EOVERFLOW, as i_blocks
 > >    doesn't fit into user-level struct.
 > >  . top sees nothing.
 > 
 > I wonder if you don't have a different problem hiding in there somewhere.
 > I was thinking that if a non-zero i_blocks field was causing problems for
 > reiserfs, maybe clear_inode() should zero this out for everyone.
 > 
 > Looking through the reiserfs code (which is rather hard to follow), I see
 > that reiserfs_cut_from_item() and prepare_for_delete_or_cut() are changing
 > i_blocks, and this is (eventually) called from reiserfs_delete_item() and
 > reiserfs_delete_object().  Not that I can point to any specific code, but
 > to me it would seem that you are not counting i_blocks correctly somewhere,
 > and eventually decrement i_blocks too much (hence -7 in i_blocks).

The code path in which symlink gets i_blocks == -7 is precisely known and
only occurs when its being deleted, so it's not a problem.

 > 
 > So this makes me believe that setting i_blocks=0 is just hiding a block
 > leak somewhere in the reiserfs code, or math errors somewhere.
 > 
 > It is also true that clear_inode() or get_new_inode() (or proc_read_inode())
 > should initialize all of the used fields to zero so that a problem in one
 > filesystem can't cause problems elsewhere.  It appears that i_blocks is

Yes, I agree with you and Alexander on this. We just wanted this glaring
bug to fixed immediately and so sent out as small patch as possible.

 > set to zero by ext2_new_inode(), so probably proc_read_inode() should do
 > the same (since it uses i_blocks).
 > 
 > Cheers, Andreas
 > 
 > PS - could someone on the reiserfs team (or Linus) run the reiserfs code
 >      through "indent" (or auto format in emacs) per Documentation/CodingStyle?
 >      It is really a gross mess, to such a point that you can hardly see what

Oh, yes.

 >      is going on.  It's not just that it is a different indent style, it has
 >      no coherent indentation or comment formatting at all.  Maybe for 2.5?

I hope so. We already have 200k of cleanup patches in 2.4.7-ac3.

 > -- 
 > Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
 >                  \  would they cancel out, leaving him still hungry?"
 > http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert

Nikita.

Re: [reiserfs-dev] Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[Hans Reiser]

Alexander Viro wrote:

> Thanks for spotting, but I have to disagree with analysis.
>         a) it's a procfs bug. We leave ->i_blocks uninitialized and inode
> constructor leaves it in undefined state.
>         b) I'd rather fix it once in fs/inode.c::clean_inode() instead of
> hunting similar bugs down again and again.
> 
> See if the patch below works for you. It makes sure that inodes passed to
> ->read_inode() or returned by new_inode() have zero in i_blocks and removes
> the redundant assignments in filesystems. Warning: it's completely untested.

Nikita sent you an email saying that he though you should fix it in the manner
you fixed it above, and then posted a patch which was localized in its impact to
just ReiserFS because he was sure it was correct.

Nikita should be more aggressive in patching VFS rather than asking you to do
it, but I don't think you can claim to be differing with his analysis.

Hans

Re: [reiserfs-dev] Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[jlnance]

On Thu, Aug 02, 2001 at 10:31:15PM +0400, Nikita Danilov wrote:
> Andreas Dilger writes:
>  > PS - could someone on the reiserfs team (or Linus) run the reiserfs code
>  >      through "indent" (or auto format in emacs) per
>  >      Documentation/CodingStyle?  It is really a gross mess, to such a
>  >      point that you can hardly see what
> 
> Oh, yes.
> 
>  >      is going on.  It's not just that it is a different indent style, it
>  >      has no coherent indentation or comment formatting at all.  Maybe
>  >      for 2.5?
> 
> I hope so. We already have 200k of cleanup patches in 2.4.7-ac3.

>From past experience, if you want to run the code through indent, I would
recomend that you submit patches for that which contain no other changes.
If you mix indent changes with code changes, it gets very difficult to
see what was actually changed if bugs crop up.

Thanks,

Jim

Re: [reiserfs-dev] Re: [PATCH]: reiserfs: D-clear-i_blocks.patch

[Nikita Danilov]

jlnance@intrex.net writes:
 > On Thu, Aug 02, 2001 at 10:31:15PM +0400, Nikita Danilov wrote:
 > > Andreas Dilger writes:
 > >  > PS - could someone on the reiserfs team (or Linus) run the reiserfs code
 > >  >      through "indent" (or auto format in emacs) per
 > >  >      Documentation/CodingStyle?  It is really a gross mess, to such a
 > >  >      point that you can hardly see what
 > > 
 > > Oh, yes.
 > > 
 > >  >      is going on.  It's not just that it is a different indent style, it
 > >  >      has no coherent indentation or comment formatting at all.  Maybe
 > >  >      for 2.5?
 > > 
 > > I hope so. We already have 200k of cleanup patches in 2.4.7-ac3.
 > 
 > >From past experience, if you want to run the code through indent, I would
 > recomend that you submit patches for that which contain no other changes.
 > If you mix indent changes with code changes, it gets very difficult to
 > see what was actually changed if bugs crop up.

Thank you. Do you have an experience in or opinion on asking Linus to
perform such conversion himself, because this will probably save some
trouble?

 > 
 > Thanks,
 > 
 > Jim

[lkml: please CC me, I am not subscribed.]

Nikita.

[PATCH]: reiserfs: D-clear-i_blocks.patch

[Nikita Danilov]

Hello, Linus,

    This patch sets inode.i_blocks to zero on deletion of reiserfs
    file. This in particular cures hard to believe bug when saving file in
    EMACS caused top to loose sight of all processes:
     . reiserfs didn't properly cleared i_blocks when removing
       symlinks. Actually -7 was inserted into unsigned i_blocks field. This
       didn't usually hurt because file is being deleted;
     . inode is reused for procfs and neither get_new_inode() nor
       proc_read_inode() cleared i_blocks;
     . now procfs inode has huge i_blocks field;
     . top calls stat on it and libc wrapper returns EOVERFLOW, as i_blocks
       doesn't fit into user-level struct.
     . top sees nothing.
   Alexander Viro and other people proposed that in stead i_blocks
   should be cleared in generic VFS code. This patch is minimal
   change required to correct bug.

This patch is against 2.4.10-pre2.
Please apply.

Nikita.
diff -rup linux/fs/reiserfs/inode.c linux.patched/fs/reiserfs/inode.c
--- linux/fs/reiserfs/inode.c	Wed Aug  1 17:21:10 2001
+++ linux.patched/fs/reiserfs/inode.c	Wed Aug  1 21:33:37 2001
@@ -55,6 +55,7 @@ void reiserfs_delete_inode (struct inode
 	;
     }
     clear_inode (inode); /* note this must go after the journal_end to prevent deadlock */
+    inode->i_blocks = 0;
     unlock_kernel() ;
 }