Re: Q: bugs in generic_forget_inode()?

[Kirill Korotaev <dev@sw.ru>]

Andrew Morton wrote:
> Kirill Korotaev <dev@sw.ru> wrote:
> 
>>Hello,
>>
>>1. I found that generic_forget_inode() calls write_inode_now() dropping 
>>inode_lock and destroys inode after that. The problem is that 
>>write_inode_now() can sleep and during this sleep someone can find inode 
>>in the hash, w/o I_FREEING state and with i_count = 0.
> 
> The filesystem is in the process of being unmounted (!MS_ACTIVE).  So the
> question is: who is doing inode lookups against a soon-to-be-defunct
> filesystem?

Yup, I'm studing this issue.
But while looking at code I found this interesting place:

__writeback_single_inode()
{
         while (inode->i_state & I_LOCK) {
                 __iget(inode);			<<<<<<
                 spin_unlock(&inode_lock);
                 __wait_on_inode(inode);
                 iput(inode);			<<<<<<
                 spin_lock(&inode_lock);
        }
	return __sync_single_inode(inode, wbc); <<<<<<
}

the problem here is iget/iput.

There are 2 possible cases:
1. all callers of this function do hold reference on inode, then 
iget/iput is unneeded.
2. if 1) is incorrect then it's a bug, since inode is used after iput.

This place looks really ugly, or I don't understand something here?

Kirill