Re: Q: bugs in generic_forget_inode()?

[Kirill Korotaev]

Andrew Morton wrote:
> Kirill Korotaev <dev@xxxxx> 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/