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