Re: [patch 12/16] fix race between writeback and unlink

Andrew Morton
2002-06-01T19:16:25Z

Linus Torvalds wrote:
>
> On Sat, 1 Jun 2002, Andrew Morton wrote:
> >
> > So run __iget prior to dropping inode_lock.
>
> This part looks horrible:
>
> + spin_unlock(&inode_lock);
> + iput(inode);
> + spin_lock(&inode_lock);

Yup. The inode refcounting APIs are really awkward. Note how I recently
added dopey code in ext2_put_inode() to only drop the prealloc window on
the "final" iput().

> Why not just split up the code inside iput(), and then just do
>
> if (atomic_dec(&inode->i_count))
> final_iput(inode);
>
> where final_iput() _wants_ the spinlock held already?
>
> That's basically what "iput()" will end up doing, except for that
> "put_inode()" thing, which is just a horrible hack anyway.
>
> So get rid of "put_inode()", and replace it with a new one that takes the
> place of the
>
> if (!inode->i_nlink) {
> ... delete ..
> } else {
> .. free ..
> }
>
> and makes that one be a "i_op->drop_inode" thing that defaults to the
> current "delete if i_nlink is zero, free it if i_nlink is not zero and
> nobody uses it".
>
> The general VFS layer really shouldn't have assigned that strogn a meaning
> to "i_nlink" anyway, it's not for the VFS layer to decide (and it only
> causes problems for any non-UNIX-on-a-disk filesystems).
>

Yes, I suspect all the inode refcounting, locking, I_FREEING, I_LOCK, etc
could do with a spring clean. Make it a bit more conventional. I'll
discuss with Al when he resurfaces.

-