Re: PATCH to pre-patch-2.1.45: clean_inode needs to reset i_writecount

Linus Torvalds (
Thu, 10 Jul 1997 16:07:02 -0700 (PDT)

On Thu, 10 Jul 1997, Bill Hawes wrote:
> The negative values in i_writecount are because the previous use of the
> inode set the field to prevent writing, so it does need to be cleared in
> clean_inode.

Right you are.

> Some other potential problems to watch out for -- there appear to be
> calls to mark_inode_dirty for pipe inodes, which puts them on the dirty
> list, then onto the clean list, but not in the hash queues. This may
> not do any harm, but it violates the design spec.

Actually, it doesn't really violate the design spec (which is essentially
to have the i_list and i_hash lists totally separate), but it _does_
violate a comment in the sources. I guess I should fix the comment ;)

> Also, I changed the code in get_new_inode and get_empty_inode to
> atomic_inc() the use count, rather than set it. The reason is that in
> the call to put_inode(), there may be cases where the code will block
> between calling clear_inode() and returning. Thus for a short time the
> inode may be one the unused list but with its count still at 1, and it's
> possible for it to go back into use. Setting the use count instead of
> incrementing would then screw up the counts.

I actually prefer the atomic_set(), because "clear_inode()" can set the
count to zero, and then the count gos negative when iput() decrements it.
But that's ok - nobody cares about the inode count once it is on the
unused list.

> In spite of a few glitches the new code seems to be working pretty well
> -- I can compile kernels, copy source trees, and generally muck around
> without too much corruption :-)

I haven't seen any corruption - the only problem I see these days is the
memory leak due to not freeing dentries (I'm working on this), and the
related dirty shutdown because we don't know that we can unmount.

This _does_ result in "dtime not zero" messages from fsck (which runs at
each boot), but that's actually because the ext2 "dtime" field is simply
broken. So you should actually not worry about that message (but anything
else would be worrysome..).